coderaiser / putout

šŸŠ Pluggable and configurable JavaScript Linter, code transformer and formatter, drop-in ESLint superpower replacement šŸ’Ŗ with built-in support for js, jsx, typescript, flow, markdown, yaml and json. Write declarative codemods in a simplest possible way šŸ˜
https://putout.cloudcmd.io/
MIT License
698 stars 40 forks source link

Wrong transform with @putout/plugin-for-of #196

Closed sirenkovladd closed 8 months ago

sirenkovladd commented 8 months ago
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 https://github.com/putoutjs/minify/issues/7

coderaiser commented 8 months ago

Thanks! Just fixed šŸŽ‰ . Please re-install šŸŠ. Is it works for you?

sirenkovladd commented 8 months ago

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)));
coderaiser commented 8 months ago

What kind of transformation you expect to see here?

sirenkovladd commented 8 months ago

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

sirenkovladd commented 8 months ago

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);
}()
coderaiser commented 8 months ago

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.

sirenkovladd commented 8 months ago

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

coderaiser commented 8 months ago

Have you tried new version?

sirenkovladd commented 8 months ago

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

coderaiser commented 8 months ago

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?

sirenkovladd commented 8 months ago

Thank you, I understand now, thanks for the solution

sirenkovladd commented 8 months ago

And could you update the minify version as well?

coderaiser commented 8 months ago

It is updated, just re-install it

sirenkovladd commented 8 months ago

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

coderaiser commented 8 months ago

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