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: support ESLint v8 #3737
feat: support ESLint v8 #3737
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 on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
That's amazing! This is the thing I wrote in #3735. The only thing that is missing is use-at-own-risk from eslint migration. You can cherry-pick from my PR. And I think |
I don't think use-at-own-risk can be used unless this plugin drops support for eslint <= v7. |
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';
import semver from 'semver';
const isESLintV8 = semver.major(version) >= 8;
export function getESLintCoreRule<R extends RuleId>(ruleId: R): RuleMap[R] {
if (isESLintV8) {
return require('eslint/use-at-own-risk').builtinRules[ruleId];
} else {
return require(`eslint/lib/rules/${ruleId}`);
}
}
// for no-loss-of-precision
export function maybeGetESLintCoreRule<R extends RuleId>(ruleId: R): RuleMap[R] | null {
try {
return getESLintCoreRule<R>(ruleId);
} catch {
return null;
}
} |
Thank you for the feedback! I changed to the method you suggested. |
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? |
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!
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.See feat: support ESLint v8 #3737 (comment).meta.hasSuggestions
to the rules that provide suggestions.no-dupe-class-members
andno-extra-semi
to work with v8.0.0-beta.0.CLIEngine
to beundefined
in eslint v8.jest-resolver.js
jest 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.
See Support package exports in
jest-resolve
jestjs/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?