-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor transform-block-scoped-function
#16398
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/57883 |
We actually have the same behavior in |
It seems we can remove
|
@liuxingbaoyu Yeah we could consider doing it for Babel 8, if |
@@ -5,7 +5,41 @@ import type { NodePath } from "@babel/traverse"; | |||
export default declare(api => { | |||
api.assertVersion(REQUIRED_VERSION(7)); | |||
|
|||
function stableSort( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Babel 8 we can use array.sort, right?
It might be worth preparing a test262 CI for Babel 8. :)
console.log(a());
function a() {
"use strict";
if (f() !== 1) return false;
function f() {
return 1;
}
{
if (f() !== 2) return false;
function f() {
return 2;
}
if (f() !== 2) return false;
}
if (f() !== 1) return false;
return true;
} |
It seems good from the test262 results. The three new failures are Another failure is because But the current test failing on node 6 is weird and I need to keep investigating. This seems to be a feature we didn't cover, it happens on node 6 but not node 8. |
babel/babel-test262-runner@2973657 Update: Just parsing options. :) |
9d3bde8
to
bf12bd7
Compare
_blockHoist
usagetransform-block-scoped-function
switch (0) {
default:
function x() {
return _x.apply(this, arguments);
}
}
x;
console.log("ok"); This succeeded on local node 6, which means that the CI failure may be caused by other reasons. (possibly older version of Jest) |
1a6deed
to
392da90
Compare
This PR is probably not suitable for landing Babel 7, it's complex and risky. For example |
path.setData( | ||
"babel-plugin-transform-async-generator-functions#async_generator_function", | ||
true, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
babel/packages/babel-plugin-transform-async-generator-functions/src/index.ts
Lines 103 to 113 in 08b0472
visitor: { | |
Program(path, state) { | |
// We need to traverse the ast here (instead of just vising Function | |
// in the top level visitor) because for-await needs to run before the | |
// async-to-generator plugin. This is because for-await is transpiled | |
// using "await" expressions, which are then converted to "yield". | |
// | |
// This is bad for performance, but plugin ordering will allow as to | |
// directly visit Function in the top level visitor. | |
path.traverse(visitor, state); | |
}, |
This is because we converted the
async generator function
in Program.enter
.
Maybe it doesn't matter if we land on Babel 7. |
cef6db9
to
32ca33a
Compare
32ca33a
to
93d09d9
Compare
Let's merge it before the next Babel 8 release? |
93d09d9
to
4b85c4f
Compare
packages/babel-plugin-transform-async-generator-functions/src/index.ts
Outdated
Show resolved
Hide resolved
…index.ts Co-authored-by: Nicolò Ribaudo <[email protected]>
Fixes #1, Fixes #2
This was introduced at #16363 and has not been released yet.
Although the behavior in that PR is more robust,
_blockHoist
is undocumented and we have removed some of its usage in the past.