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

feat(ts-estree): add preserveNodeMaps option #494

Merged
merged 2 commits into from May 5, 2019
Merged

Conversation

JamesHenry
Copy link
Member

@JamesHenry JamesHenry commented May 5, 2019

I have been working on converting some user-land custom TSLint rules to ESLint and this feature has come from that work.

  • In many cases, I am discovering migrating custom TSLint rules to ESLint is far easier if you first migrate the "infrastructure" over (all the rule and RuleTester config), but keep the rule logic largely the same. (With the idea that you can truly migrate the implementation at some point later).

  • Naturally, this is only possible if you have access to the two-way AST node maps that may be preserved during conversion.

  • Preserving the AST node maps is currently only possible if using the project option. This is a good start, but as we know the project feature is expensive because of its requirement to create full TypeScript Programs.

  • If a custom TSLint rule only requires syntactic information, it is completely unnecessary to create a full Program.

This PR introduces a new option for typescript-estree which allows for the user to explicitly control whether or not AST node maps are preserved during conversion.

There will be follow up PRs where I add utilities I have created for ESLint rules which meet the above criteria (e.g.getNodeMaps()), but I wanted to add the flag itself in a dedicated PR.

(NOTE: for backwards compatibility the existing behaviour around project causing node maps to be preserved has been maintained here)

@codecov
Copy link

codecov bot commented May 5, 2019

Codecov Report

Merging #494 into master will increase coverage by 0.06%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
+ Coverage   95.75%   95.81%   +0.06%     
==========================================
  Files          77       77              
  Lines        3484     3489       +5     
  Branches      960      963       +3     
==========================================
+ Hits         3336     3343       +7     
  Misses         51       51              
+ Partials       97       95       -2
Impacted Files Coverage Δ
packages/typescript-estree/src/ast-converter.ts 100% <100%> (ø) ⬆️
packages/typescript-estree/src/convert.ts 98.96% <100%> (ø) ⬆️
packages/typescript-estree/src/parser.ts 95% <88.88%> (+2.01%) ⬆️

@JamesHenry JamesHenry merged commit c3061f9 into master May 5, 2019
@JamesHenry JamesHenry deleted the preserve-node-maps branch May 5, 2019 21:09
@JamesHenry
Copy link
Member Author

@j-f1 Taking your thumbs up emojis on here and slack as an approval review 😛

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant