Skip to content

Support ESTree AccessorProperty #17082

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Mar 21, 2025
Merged

Support ESTree AccessorProperty #17082

merged 8 commits into from
Mar 21, 2025

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jan 23, 2025 •
edited
Loading

Q                       A
Fixed Issues? (part of #16679), fixes #15188
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

The AccessorProperty is part of the stage 3 ESTree decorators spec, which is already supported by typescript-eslint.

@JLHwung JLHwung added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Jan 23, 2025
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 23, 2025 •
edited
Loading

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

nicolo-ribaudo
nicolo-ribaudo reviewed Jan 29, 2025
View reviewed changes
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.

For class properties, we didn't enable their ESTree AST by default in the parser because there was a high risk of people relying on the old AST. Do you think there is a similar risk? What should we do if classFeatures of the ESTree plugin is not set to true?

@JLHwung
Copy link
Contributor Author

JLHwung commented Jan 31, 2025

For class properties, we didn't enable their ESTree AST by default in the parser because there was a high risk of people relying on the old AST. Do you think there is a similar risk? What should we do if classFeatures of the ESTree plugin is not set to true?

Good question. IIRC the classFeatures option was introduced before ClassAccessorProperty was supported in the parser (the 2021-12 decorator version), since then it is not discussed whether classFeatures should cover other class proposals. For the time being, let's keep it behind the classFeatures flag.

For any new proposals, we should forward the AST design to the ESTree and implement them in the estree plugin right away if they are revised from our own AST, so we don't have to introduce similar feature flags in the future.

nicolo-ribaudo
nicolo-ribaudo approved these changes Feb 1, 2025
View reviewed changes
eslint/babel-eslint-plugin/src/rules/no-undef.cjs Outdated
node.parent.key === node &&
!node.parent.computed
(parent.type === "AccessorProperty" ||
parent.type === "ClassAccessorProperty") &&
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this second check for BABEL_8_BREAKING

@nicolo-ribaudo nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Feb 2, 2025
@JLHwung JLHwung added this to the v7.27.0 milestone Feb 21, 2025
@liuxingbaoyu
Copy link
Member

Does this also fix #15188 ?

@nicolo-ribaudo nicolo-ribaudo merged commit 10c4bd4 into babel:main Mar 21, 2025
56 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the estree-accessor-property branch March 21, 2025 09:53
laine-hallot pushed a commit to laine-hallot/uwu-parser that referenced this pull request Mar 31, 2025
@JLHwung
Support ESTree AccessorProperty (babel#17082)
78c4e9b 
* feat: create AccessorProperty for ESTree

* add new test cases

* add private key test case

* update Babel 8 test fixtures

* support AccessorProperty in eslint-*

* guard accessor property rename by classFeatures

* update parser options schema

* address review comments
@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 Jun 21, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

@liuxingbaoyu liuxingbaoyu liuxingbaoyu approved these changes

Assignees
No one assigned
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release
Projects
None yet
Milestone
v7.27.0
Development

Successfully merging this pull request may close these issues.

Inconsistent AST type to ESTree for decorator auto access syntax
4 participants
@JLHwung @babel-bot @liuxingbaoyu @nicolo-ribaudo