Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat: support ESLint v8#3737
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for the PR,@ota-meshi! 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 onhttps://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day. |
nx-cloudbot commentedAug 15, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
coderaiser commentedAug 15, 2021
That's amazing! This is the thing I wrote in#3735. The only thing that is missing isuse-at-own-risk fromeslint migration. You can cherry-pick from my PR. And I think |
Uh oh!
There was an error while loading.Please reload this page.
ota-meshi commentedAug 15, 2021
I don't think use-at-own-risk can be used unless this plugin drops support for eslint <= v7. |
bradzacher commentedAug 16, 2021
It's possible for us to use both - it just means we can't migrate to ESM any time soon (not that we were planning on doing that. There's only one module we depend on that's made that switch so far). This example code should work and allow us to support both, I believe: import{version}from'eslint';importsemverfrom'semver';constisESLintV8=semver.major(version)>=8;exportfunctiongetESLintCoreRule<RextendsRuleId>(ruleId:R):RuleMap[R]{if(isESLintV8){returnrequire('eslint/use-at-own-risk').builtinRules[ruleId];}else{returnrequire(`eslint/lib/rules/${ruleId}`);}}// for no-loss-of-precisionexportfunctionmaybeGetESLintCoreRule<RextendsRuleId>(ruleId:R):RuleMap[R]|null{try{returngetESLintCoreRule<R>(ruleId);}catch{returnnull;}} |
ota-meshi commentedAug 16, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thank you for the feedback! I changed to the method you suggested. |
ota-meshi commentedAug 16, 2021
Do you have any ideas on how to fix a lint crash? What do you think about changing to using eslint v7 in devDependencies and using eslint v8 only for CI unit tests? |
bradzacher left a comment
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 pushed a fix for the broken linting - this was due to breaking changes not being supported yet. For now I've manually patched the packages to handle this. Eventually they'll release full support and we can drop those patches.
LGTM - thanks for this!
Uh oh!
There was an error while loading.Please reload this page.
This PR updates this plugin to support ESLint v8.
However, currently eslint is still v8.0.0-beta-0.
Summary of changes
Change to get ESLint core rules from Linter instance.Seefeat: support ESLint v8 #3737 (comment).meta.hasSuggestionsto the rules that provide suggestions.no-dupe-class-membersandno-extra-semito work with v8.0.0-beta.0.CLIEngineto beundefinedin eslint v8.jest-resolver.jsjest doesn't seem to support the exports map in package.json at this time.
For this reason, jest-resolver.js is added as a workaround.
SeeSupport package exports in
jest-resolvejestjs/jest#9771.About Lint errors:
Since eslint-plugin-import and eslint-plugin-jest do not yet support eslint v8, still get a run error.
Test failed on Node 10.x:
Test fails because eslint v8 doesn't support Node 10.
Should I change the problematic workflow and use eslint v7 only for Node 10.x testing and linting? Or should I delete the workflow for Node 10.x?