Skip to content
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

Fix optional chaining bug regarding spread in function calls #8210

Closed
wants to merge 1 commit into from

Conversation

microbouji
Copy link
Member

Q                       A
Fixed Issues? Fixes #8136
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR
Any Dependency Changes?
License MIT

Currently when proposal-optional-chaining transforms

a?.b()

into

(_a = a) === null || _a === void 0 ? void 0 : _a.b()

that last _a.b() is technically an OptionalCallExpression with no optional field set.

This PR changes it to be just a CallExpression. I believe this is how it should be, and this also enables other plugins (like transform-spread) who use CallExpression in their visitors to keep working.

@babel-bot
Copy link
Collaborator

babel-bot commented Jun 22, 2018

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8452/

@hzoo hzoo requested a review from jridgewell June 22, 2018 14:49
@hzoo hzoo added Spec: Optional Chaining PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Jun 22, 2018
@@ -83,6 +83,10 @@ export default declare((api, options) => {
}
}

if (replacementPath.isOptionalCallExpression()) {
replacementPath.node.type = "CallExpression";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, the type should be changed in the transform for sure? This logic is similar to what was done in https://github.com/babel/babel/pull/8210/files#diff-accf2589ebb96f2b32131e0a1f8a08eaR24

if (objectPath.isOptionalMemberExpression()) {
  objectPath.node.type = "MemberExpression";
}

@dackmin
Copy link

dackmin commented Nov 22, 2018

@hzoo Do you have an idea of when this PR will be merged & released ? I came across this issue using a spread operator into an optional chaining method call and the spread operator is invisible for @babel/plugin-transform-spread.

@jridgewell
Copy link
Member

jridgewell commented Nov 24, 2018

Closing this for #9073, which fixes the more general case. I've reused your commits, so you'll still get credit. Thanks for starting it!

@jridgewell jridgewell closed this Nov 24, 2018
@microbouji microbouji deleted the patch/8136 branch November 26, 2018 16:08
@microbouji
Copy link
Member Author

@jridgewell thank you!

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Optional Chaining
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spread-operator doesn't transform to ES5
5 participants