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 yield await classification #12230
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/30997/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 46ba92f:
|
15773bb
to
ae9acf0
Compare
// verify if the same logic is needed for yield. | ||
if (oldYieldPos !== -1) this.state.yieldPos = oldYieldPos; | ||
|
||
// Await is trickier than yield. When parsing a possible arrow function |
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.
@nicolo-ribaudo I really love reading these informative comments.
...ser/test/fixtures/es2015/yield/parameter-default-inside-arrow-inside-generator-5/output.json
Outdated
Show resolved
Hide resolved
@@ -2,7 +2,7 @@ | |||
"type": "File", | |||
"start":0,"end":35,"loc":{"start":{"line":1,"column":0},"end":{"line":3,"column":1}}, | |||
"errors": [ | |||
"SyntaxError: Can not use 'yield' as identifier inside a generator (2:3)", | |||
"SyntaxError: yield is not allowed in generator parameters (2:3)", |
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.
Lines 1 to 3 in d51aa6d
function* fn() { | |
(x = yield) => {}; | |
} |
The error message seems a bit misleading as it is not a generator, although it is indeed in function parameters. V8 has changed this error message to "Yield expression not allowed in formal parameter" (code), I suggest we align with V8 and update the error in following PRs.
@@ -2,7 +2,7 @@ | |||
"type": "File", | |||
"start":0,"end":36,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":36}}, | |||
"errors": [ | |||
"SyntaxError: Can not use 'await' as identifier inside an async function (1:20)" | |||
"SyntaxError: Can not use 'await' as identifier inside an async function (1:14)" |
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.
Line 1 in d51aa6d
async (a = ({ await }) => {}) => {}; |
The position of await
should be 14. An unexpected bugfix. 😄
b0f87cf
to
8c92bd9
Compare
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
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.
This is awesome 🎉
Currently
@babel/parser
uses parser state to track the position and possible declaration error ofyield
andawait
in ambiguous patterns: arrow head ((x)
) and async arrow headasync(x)
. Historically we have done several fixing attempts (#6877, #7727, #9405, #10469, #11148, etc.), however this approach is error-prone and can not handle edge cases especially if these patterns are nested: For example, many, if not most, test cases I added in this PR are failing onmain
but fixed by this PR.We introduce an
ExpressionScope
to handle declaration errors in ambiguous patterns. See here for the design ofExpressionScope
. The idea is to record such errors and only thrown at the time when such pattern is indeed an arrow function or an async arrow function. The stack based scope play nicely with nested structures: so errors will not be thrown too early or incorrectly reset when later a new structure is parsed.