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

Handle private access chained on an optional chain #11248

Merged

Conversation

jridgewell
Copy link
Member

@jridgewell jridgewell commented Mar 12, 2020

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

See the resolution from tc39/proposal-class-fields#301.

We intend to allow devs to access private properties in the optional chains, eg foo?.bar.#baz. We're still debating whether to allow the private property to start the optional chain, eg foo?.#baz.

--- Edits By @JLHwung ---

Todo list tracking ?. PrivateIdentifier support

  • foo?.#x
  • foo?.#m()
  • foo.#m?.()
  • foo?.#m?.()
  • add test cases for loose mode

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

This is the most important comment: #11248 (comment)

jridgewell added a commit to jridgewell/babel that referenced this pull request Mar 13, 2020
This is a mix of changes, most importantly:

- Always print parens when mixing nullish coalescing and another logical
- Fixes assignment in the callee of an optional chain, which now blocks babel#11248

Also, cleans up a bit of code, and removes an unnecessary parens around `class A extends new B {}`
nicolo-ribaudo pushed a commit that referenced this pull request Mar 13, 2020
This is a mix of changes, most importantly:

- Always print parens when mixing nullish coalescing and another logical
- Fixes assignment in the callee of an optional chain, which now blocks #11248

Also, cleans up a bit of code, and removes an unnecessary parens around `class A extends new B {}`
@jridgewell jridgewell force-pushed the optional-private branch 2 times, most recently from fa21565 to 54b2109 Compare March 15, 2020 08:48
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Thanks!

@nicolo-ribaudo nicolo-ribaudo added this to the v7.10.0 milestone Mar 22, 2020
@babel-bot
Copy link
Collaborator

babel-bot commented May 20, 2020

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 20, 2020

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 6c4486f:

Sandbox Source
compassionate-leftpad-exj48 Configuration
distracted-knuth-q0ol1 Configuration

@babel babel deleted a comment from codesandbox-ci bot May 20, 2020
@JLHwung JLHwung mentioned this pull request May 21, 2020
@JLHwung JLHwung marked this pull request as draft May 21, 2020 15:51
@JLHwung
Copy link
Contributor

JLHwung commented May 21, 2020

I am converting this PR to draft as transforming ?. PrivateIdentifier is still WIP.

@nicolo-ribaudo
Copy link
Member

Whops I clicked "Approve" on the wrong PR

@JLHwung JLHwung force-pushed the optional-private branch 3 times, most recently from 397d516 to 9635a42 Compare May 22, 2020 15:34
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
@JLHwung JLHwung marked this pull request as ready for review May 25, 2020 17:15
@JLHwung
Copy link
Contributor

JLHwung commented May 25, 2020

✅On changes by @jridgewell . But this PR needs approval other than myself on my changes.

Thanks for @nicolo-ribaudo who inspires me on the #m?.() support!

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Thanks for figuring out the #m?.() after I offered my help and then failed 😂

(_getSelf6 = (_ref22 = fn === null || fn === void 0 ? void 0 : _classPrivateFieldLooseBase(fn().Foo, _self)[_self]) === null || _ref22 === void 0 ? void 0 : (_ref22$getSelf = _ref22.getSelf) === null || _ref22$getSelf === void 0 ? void 0 : _ref22$getSelf.call(_ref22)) === null || _getSelf6 === void 0 ? void 0 : _classPrivateFieldLooseBase(_getSelf6.self, _m)[_m]();
(_call2 = (_ref20 = fn === null || fn === void 0 ? void 0 : (_classPrivateFieldLoo4 = _classPrivateFieldLooseBase(fn().Foo, _self)[_self]).getSelf) == null ? void 0 : _ref20.call(_classPrivateFieldLoo4)) === null || _call2 === void 0 ? void 0 : _classPrivateFieldLooseBase(_call2.self, _m)[_m]();
(_getSelf5 = (_ref21 = fn === null || fn === void 0 ? void 0 : _classPrivateFieldLooseBase(fn().Foo, _self)[_self]) == null ? void 0 : _ref21.getSelf()) === null || _getSelf5 === void 0 ? void 0 : _classPrivateFieldLooseBase(_getSelf5.self, _m)[_m]();
(_getSelf6 = (_ref22 = fn === null || fn === void 0 ? void 0 : _classPrivateFieldLooseBase(fn().Foo, _self)[_self]) == null ? void 0 : _ref22.getSelf == null ? void 0 : _ref22.getSelf()) === null || _getSelf6 === void 0 ? void 0 : _classPrivateFieldLooseBase(_getSelf6.self, _m)[_m]();
Copy link
Member

Choose a reason for hiding this comment

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

In the future we'll figure out how to use == null here.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could add loose support to

export default function memberExpressionToFunctions(path, visitor, state) {

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

@jridgewell some fantastic code comments in this PR!

@nicolo-ribaudo nicolo-ribaudo merged commit a0747ae into babel:feat-7.10.0/private-stuff May 26, 2020
@jridgewell jridgewell deleted the optional-private branch May 26, 2020 20:18
nicolo-ribaudo added a commit that referenced this pull request May 26, 2020
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: Huáng Jùnliàng <jlhwung@gmail.com>
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Aug 26, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 26, 2020
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 pkg: parser PR: New Feature 🚀 A type of pull request used for our changelog categories Spec: Class Fields Spec: Optional Chaining Spec: Private Methods
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants