coderaiser/putout

Wrong transform with @putout/plugin-for-of

sirenkovladd opened this issue · 15 comments

import {
  parse,
  transform,
} from 'putout';
import { print } from '@putout/printer';
import forOf from '@putout/plugin-for-of';

const js = `
a.forEach(aa=>d.push(aa)),
b.forEach(bb=>d.push(bb))`;

const ast = parse(js, {
  printer: 'putout',
});

let raw = print(ast);
console.log(raw)

console.log('----------------------')

transform(ast, js, {plugins: [['for-of', forOf]]});

raw = print(ast);
console.log(raw)
❯ node tt.js
(a.forEach((aa) => d.push(aa)), b.forEach((bb) => d.push(bb)));

----------------------
function(_ret) {
    for (const bb of b)
        _ret = d.push(bb);
    
    return _ret;
}();

Missing code for a.forEach(aa=>d.push(aa))

related to putoutjs/minify#7

Thanks! Just fixed 🎉 . Please re-install 🐊. Is it works for you?

It's better now, but I don't see any transformation and the forEach still exists

❯ node tt.js
(a.forEach((aa) => d.push(aa)), b.forEach((bb) => d.push(bb)));

----------------------
(a.forEach((aa) => d.push(aa)), b.forEach((bb) => d.push(bb)));

What kind of transformation you expect to see here?

in my opinion, it should be the most appropriate

(function() {
    for (const aa of a)
        d.push(aa);
}(),
function() {
    for (const bb of b)
        d.push(bb);
}())

can correct me if you think otherwise

not sure if this applies to this task but I'll write here first
for code

const js = `const z=a.forEach(aa=>d.push(aa))`;

result

 node tt.js
const z = a.forEach((aa) => d.push(aa));

----------------------
const z;

I expect something like

const z = function() {
    for (const bb of b)
        d.push(bb);
}()

Just fixed 🎉 . Please re-install 🐊. Is it works for you?

Here is a new rule for merging loops: minify/merge-loops.

About const z=a.forEach(aa=>d.push(aa)), forEach() returns nothing so better to keep this code as is.

But it removes the loop and without minification it causes a push
and after that it doesn't do anything

Have you tried new version?

Oh yes, the version was not the last
now I have a row without changes
I just thought that plugin-for-of must strictly change to for-of

but if it is skipped for minification then it seems it should just use a more complex condition than just replacing or skipping it

Look, @putout/plugin-for-of used both in Putout linter and Minify, so when trapped on SequenceExpression it just skippes transformation, since we have @putout/plugin-extract-sequence-expression which will do the thing first.

About your case:

a.forEach(aa=>d.push(aa)),
b.forEach(bb=>d.push(bb))`;

Now with help of @putout/plugin-minify it will be transformed to:

for (const aa of [...a, ...b]) {
    d.push(aa);
}

And then all whitespaces will be reduced.

it seems it should just use a more complex condition than just replacing or skipping it

What do you suggest?

Thank you, I understand now, thanks for the solution

And could you update the minify version as well?

It is updated, just re-install it

I see that the last version was a month ago
https://www.npmjs.com/package/minify?activeTab=versions

I’m using semantic versioning, just re-install minify and it will update all dependencies.