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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent AST type to ESTree for decorator auto access syntax #15188

Open
1 task done
sosukesuzuki opened this issue Nov 12, 2022 · 3 comments
Open
1 task done

Inconsistent AST type to ESTree for decorator auto access syntax #15188

sosukesuzuki opened this issue Nov 12, 2022 · 3 comments

Comments

@sosukesuzuki
Copy link
Member

馃捇

  • Would you like to work on this feature?

What problem are you trying to solve?

Inconsistent AST node for decorator auto accessors between ESTree and Babel.

ESTree: AccessorProperty
Babel: ClassAccessorProperty

ref: https://github.com/estree/estree/blob/master/stage3/decorators.md#accessorproperty

Describe the solution you'd like

Implement convert ClassAccessorProperty to AccessorProperty in estree plugin of babel.

Describe alternatives you've considered

Babel drops ClassAccessorProperty in favor of AccessorProperty.

Documentation, Adoption, Migration Strategy

Decorator auto accessors syntax will be landed in TypeScript 4.9 (https://devblogs.microsoft.com/typescript/announcing-typescript-4-9-rc/#auto-accessors-in-classes). Thus, the inconsistency with typescript-estree is a concern to me.

@JLHwung
Copy link
Contributor

JLHwung commented Nov 14, 2022

I don't think we will implement the estree spec until ESLint supports AccessorProperty. In the past implementing stage 3 features before ESLint support has caused compatibility issues like #12864.

Alternatively we can implement behind a feature flag like classFeatures for class properties and then materialize it in Babel 8.

@bradzacher
Copy link
Contributor

@JLHwung with class properties there was no ESTree spec for them prior to them reaching stage4
which meant that most parsers just used babel's representation (ClassProperty / ClassPrivateProperty), which caused churn in the ecosystem when there was finally the ESTree spec (PropertyDefinition).

But now ESTree is committing to supporting stage3 features and including ASTs for them (which you yourself keep adding!) - there is now a peer-reviewed spec for these (almost) stable features.

So it is safe to implement them early.
For reference - @typescript-eslint will be implementing using the stage3 spec linked.

@JLHwung
Copy link
Contributor

JLHwung commented Nov 18, 2022

I see no differences to the ClassProperty scenario. Although ESTree has supported more experimental proposals than before, there aren't any popular ESTree-experimental parser out there (correct me if I am wrong). So ESLint plugin authors have to stick with @babel/eslint-parser and thus the Babel AST if they want to support accessor property. Now if we change the AST structure before ESLint supports it, these plugin will break because they are not aware of the ESTree spec.

Now if ESLint supports it, we are more than happy to align with the ESTree proposal, and plugin authors will surely need to migrate from Babel AST to ESTree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants