-
-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: Resolve getAncestors
and getScope
calls in eslint v9
#466
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #466 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 24 -1
Lines 649 656 +7
Branches 250 250
=========================================
+ Hits 649 656 +7 ☔ View full report in Codecov by Sentry. |
getAncestors
and getScope
calls in eslint v9
/** | ||
* @fileoverview Helpers for tests. | ||
* @author 唯然<weiran.zsd@outlook.com> | ||
*/ | ||
'use strict' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was originally 'borrowed' from eslint-community/eslint-plugin-n#161, then I made a handful of changes,
I can remove the header if prefered!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also update the ci to test eslint v9?
} | ||
|
||
if (config.parserOptions) { | ||
Object.assign(config.languageOptions, config.parserOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please note parserOptions
is not 100% same as languageOptions
:
- https://eslint.org/docs/v8.x/use/configure/language-options#specifying-parser-options
- https://eslint.org/docs/latest/use/configure/language-options#specifying-javascript-options
Since this is only used for this project, it is not required to cover 100% of cases. so, it's be fine if you were not using other options. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 I was actually thinking about this, and there are a couple of things I could change:
- I am considering making a module for this rule-tester compat package of some sort so it can be used in more places
- It may be best to migrate the config backwards, not forwards. The more I think about it the less I like the
v8
->v9
compat, and instead prefer the idea ofv9
->v8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also see the value for rule authors want to support eslint v8 & eslint v9. maybe support both - auto-detecting eslint version and the config format:
- eslint v8 + eslintrc : no change
- eslint v8 + flat : flat => eslintrc
- eslint v9 + eslintrc: eslintrc -> flat
- eslint v9 + flat: no change
ESLint v9 is not actually fixed in this PR 👀 I want to do this lovely little deprecation in a different PR.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Waiting a few days to see if others want to review.
chore: Clean up config migrations Co-authored-by: 唯然 <weiran.zsd@outlook.com>
1147fe2
to
9a9eec3
Compare
What is the purpose of this pull request?
This is phase 1 of getting eslint v9 working.
The features this implements are:
getAncestors
getScope
RuleTester
class to be usable in both eslint v7, v8 and v9.As you may be able to tell, I copied a few files over from eslint-plugin-n (Specifically from eslint-community/eslint-plugin-n#161) (@aladdin-add Thank you for the prior art 🙏)
This is one part of #449.