-
Notifications
You must be signed in to change notification settings - Fork 46.4k
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
eslint-plugin-react-hooks: Add support for ESLint v9 #28773
Conversation
80e7b49
to
c5b335e
Compare
Comparing: 699d03c...c1f1de3 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
43a0927
to
1a217e9
Compare
return context.sourceCode.getScope(node); | ||
} | ||
} | ||
|
||
const scopeManager = context.getSourceCode().scopeManager; |
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.
https://eslint.org/blog/2023/09/preparing-custom-rules-eslint-v9/#context-methods-becoming-properties
I noticed that this context.getSourceCode()
was replaced with context.sourceCode
, you might want to confirm that.
const sourceCode = context.sourceCode ?? context.getSourceCode();
const scopeManager = sourceCode.scopeManager;
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.
getSourceCode
was only deprecated.
88687c4
to
e2da44f
Compare
yarn.lock
Outdated
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.
Maybe split this out into a separate 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.
It kind of doesn't make sense for these change to stay if we revert ESlint v9 support. And if we need to revert the lockfile changes, we also need to revert ESLint v9 support.
Merge conflicts in yarn.lock are not an issue since you'd just run yarn
. Yarn automatically resolves merge conflicts.
I rebased this PR to isolate these changes so if we do need to bisect later, we can still do it on this branch.
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.
ah right, sorry I missed that there was a package.json update
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
Outdated
Show resolved
Hide resolved
Will be used to test compat between eslint-plugin-react-hooks and ESLint v9
e2da44f
to
5d30424
Compare
…iveDeps-test.js Co-authored-by: lauren <poteto@users.noreply.github.com>
When will this be released? |
Still getting |
Pending on facebook/react#28773 Note - once we go eslint 9, we won't need the `--report-unused-disable-directives` in our eslint invocation: https://eslint.org/docs/latest/use/configure/configuration-files#reporting-unused-disable-directives
This PR is released in |
So how to config/use it? |
@dzcpy |
@eps1lon Can we have a stable release of the eslint plugin without waiting for React 19? |
Any updates? Still waiting on this for our stack's ESLint 9 upgrade. |
yarn add eslint-plugin-react-hooks@5.1.0-rc-eb3ad065-20240822 -D |
when can we expect a release? |
Based on #28774
Does not add support for flat configs i.e. does not adress #28313.
The PR only adds support for ESLint v9 with the old config format and sets up testing infra for separate ESLint versions. The only breaking changes that affected the plugin was around removal of
context.getScope
andcontext.getSource
which have simple replacements in V9 so that we don't have to fork too much to be able to support ESLint V9 and earlier.The last commit makes it so that we have no breaking changes. If we decide to drop support for ESLint < 9.x, we can just revert
80e7b49
(#28773)Alternate to #28772