-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Optional Chaining: Account for document.all #6525
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/5438/ |
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.
If we do it here only in spec mode, the same thing (introducing loose
mode) imho should be done in babel-plugin-transform-nullish-coalescing-operator
Indeed. I can do that in a follow-up PR. |
Sure, I didn't mean it should go into this one :) |
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.
Please add a loose test.
@jridgewell It is already there, it just isn't shown in this PR because its output isn't modified. https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-optional-chaining/test/fixtures/general/memoize-loose |
Account for
document.all
with the optional-chaining operator.Input:
Output - Before:
Output - After:
This is spec-compliant as per 3.8.1 Runtime Semantics: Evaluation
As this can generate a fair amount of code, and due to the rarity of
document.all
in practice, I've only made the change in spec mode.NB: This will likely have merge conflicts with the files to be deleted in #6345 @jridgewell