-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
[Babel 8]: Fix incorrect LVal coverage #17387
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
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/59583 |
3c116c5 to
71c0d33
Compare
|
Docs PR is ready, ptal: babel/website#3103 |
|
|
||
| function flattenLVal( | ||
| path: NodePath<t.LVal | t.OptionalMemberExpression>, | ||
| path: NodePath<t.LVal | t.PatternLike | t.OptionalMemberExpression>, |
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.
OptionalMemberExpression should probably be also an LVal itself just like MemberExpression is, rather than listing it separately?
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.
Yes. That will be a new feature and thus be handled in a new PR. (Breaking changes can be merged directly as long as it's behind the BABEL_8_BREAKING flag but features has to be waited for the next release).
In this pull request we remove
AssignmentPatternandRestElementfrom theLValalias, since they must be nested within a pattern or a function parameter, rather than being allowed to be the LHS of an assignment. The typing usage in the codebase is also updated: we broaden the previoust.LValwitht.PatternLikesuch that it covers those two elements as well.