Reduce transform-block-scoping loops output size
#15746
Conversation
liuxingbaoyu
added
the
PR: Output optimization 🔬
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/54894/ |
|
Do we have a test with multiple labels? outer: while (t) {
inner: while (t2) {
for (let i = 0; i < 3; i++) {
later(() => i);
if (test1) break outer;
else if (test2) break inner;
else if (test3) continue outer;
else if (test4) continue inner;
else if (test5) break;
else if (test6) continue;
}
}
} |
There might be something similar in the execution test, I added it in the outputs. |
| const declarations = varPath.get("declarations"); | ||
| declarations[declarations.length - 1] | ||
| .get("init") | ||
| .unwrapFunctionEnvironment(); |
There was a problem hiding this comment.
We can simplify this back to what it was if we push the new variables to the end of varPath instead of unshifting them.
There was a problem hiding this comment.
Do you think it would look better if we put the variable first?
There was a problem hiding this comment.
It would look similar, but it's slightly simpler code on our end :)
There was a problem hiding this comment.
Sounds very reasonable!
|
|
||
| var gen5 = foo3(); | ||
| expect(gen5.next().value).toBe("iteration"); | ||
| expect(gen5.next({what: "three"}).done).toBe(true); |
There was a problem hiding this comment.
Can you duplicate this test to an executable test case (exec.js)?
There was a problem hiding this comment.
Oops, this is actually a copy of packages\babel-core\test\fixtures\transformation\regenerator\destructuring\exec.js. I put it here for easy fix, maybe let me just add a note to clarify?
There was a problem hiding this comment.
In that case can you move the packages\babel-core\test\fixtures\transformation\regenerator\destructuring\exec.js to packages/babel-plugin-transform-block-scoping/test/fixtures/regression/complex? They should have been moved to plugin specific test folders.
There was a problem hiding this comment.
From what I understand it is ideally positioned?🤔
There was a problem hiding this comment.
The test cases here were added long before Babel plugins are separated from Babel core: https://github.com/babel/babel/blob/d4debc3c855dd4e797de3dbe8bcffe47d325dcb3/test/fixtures/transformation/regenerator/destructuring/exec.js
Since then most test cases have been moved to plugins. IMO the babel-core test cases should focus on core's behaviour, like providing plugin pass to plugin visitors, defining NodePath#hub or merging configs. In this case we already copy this test to the plugin's test folder (as input/output tests) and we generally do not separate exec.js with input.js. So I think the exec.js should be moved, too.
I can open a PR to move all such cases out of babel-core to avoid such confusing test fixtures structures.
|
It occurred to me, is it possible for us to support |
|
Well, in new node versions you can do |
|
Could you rebase, since we merged #15766? |
nicolo-ribaudo
force-pushed
the
improve-loop-scoping
branch
from
6448b4f
to
9dbc67e
Compare
nicolo-ribaudo
changed the title
transform-block-scoping performance and size for loopstransform-block-scoping loops output size
|
@nicolo-ribaudo Thank you! Sorry I forgot it. |
github-actions
bot
added
the
outdated
Fixes #1, Fixes #2Improved performance and size of
transform-block-scopingfor loops.There will be a small impact on the readability of the output when the comments are not preserved.