feat: support Auto Accessor syntax - #5926
Conversation
|
Thanks for the PR, @sosukesuzuki! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
# Conflicts: # package.json # patches/typescript+4.8.3.patch # patches/typescript+4.9.2-rc.patch # patches/typescript+4.9.3.patch # yarn.lock
|
Clarifying ESTree naming consistency before we move forward on this - estree/estree#292 |
|
Looks like babel doesn't support type annotations on auto-accessors: babel/babel#15205 |
|
In 3c326c4 I added tooling to the ast-spec fixture tester which allows us to add config for specific fixtures. For context, changes I have made to this PR:
This PR is waiting for a decision on naming alignment discussion with ESTree (estree/estree#292) |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #5926 +/- ##
==========================================
- Coverage 91.28% 91.24% -0.04%
==========================================
Files 366 366
Lines 12363 12380 +17
Branches 3617 3621 +4
==========================================
+ Hits 11285 11296 +11
- Misses 768 774 +6
Partials 310 310
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
The AST is staying named as is, so no changes needed there. Just need to add scope analysis support then this will be good to go. |
# Conflicts: # .prettierignore
| * The value should be a description of why there isn't support - for example a github issue URL. | ||
| */ | ||
| readonly expectBabelToNotSupport?: string; | ||
| } |
| const type = (() => { | ||
| if (isAccessor) { | ||
| if (isAbstract) { | ||
| return AST_NODE_TYPES.TSAbstractAccessorProperty; |
There was a problem hiding this comment.
I'm confused why codecov thinks this isn't covered by tests. I can debug it locally to see the TSAbstractAccessorProperty and AccessorProperty cases being hit. Though, sourcemaps seem to not be working - maybe that's the issue?
There was a problem hiding this comment.
it might be because the tests are in ast-spec instead of in here?
maybe we need to adjust how the coverage is collected?
| readonly // require keys for all nodes NOT defined in `eslint-visitor-keys` | ||
| [T in Exclude< | ||
| AST_NODE_TYPES, | ||
| KeysDefinedInESLintVisitorKeysCore | ||
| >]: readonly GetNodeTypeKeys<T>[]; | ||
| } & { | ||
| readonly // optionally allow keys for all nodes defined in `eslint-visitor-keys` | ||
| [T in KeysDefinedInESLintVisitorKeysCore]?: readonly GetNodeTypeKeys<T>[]; |
There was a problem hiding this comment.
oooooof i didn't see that prettier formatted like this
disgusting.
JoshuaKGoldberg
left a comment
JoshuaKGoldberg
left a comment
There was a problem hiding this comment.
I played around with this a bunch locally and it works great. Additional 🌟 for the testing infrastructure cleanups. This was also a great excuse for me to get more familiar with #6065 😄
🚢 !
|
Thank you!! |
|
Hello! |
4 participants
PR Checklist
Overview
Based on #5716
Supports Auto Accessors.
ref: ESTree definition