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

eslint-plugin-react-hooks: Add support for ESLint v9 #28773

Merged
merged 6 commits into from Apr 23, 2024

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Apr 6, 2024

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 and context.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

@react-sizebot
Copy link

react-sizebot commented Apr 6, 2024

Comparing: 699d03c...c1f1de3

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 497.71 kB 497.71 kB = 88.93 kB 88.93 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 504.00 kB 504.00 kB = 89.95 kB 89.95 kB
facebook-www/ReactDOM-prod.classic.js = 591.14 kB 591.14 kB = 103.91 kB 103.91 kB
facebook-www/ReactDOM-prod.modern.js = 566.95 kB 566.95 kB = 100.12 kB 100.12 kB
test_utils/ReactAllWarnings.js Deleted 64.80 kB 0.00 kB Deleted 16.14 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js +1.19% 76.83 kB 77.75 kB +0.90% 14.07 kB 14.20 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js +1.19% 76.83 kB 77.75 kB +0.90% 14.07 kB 14.20 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js +1.16% 78.69 kB 79.60 kB +0.94% 14.31 kB 14.45 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +1.01% 93.66 kB 94.60 kB +0.61% 21.96 kB 22.09 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +1.01% 93.66 kB 94.60 kB +0.61% 21.96 kB 22.09 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +1.00% 93.81 kB 94.75 kB +0.61% 21.98 kB 22.12 kB
test_utils/ReactAllWarnings.js Deleted 64.80 kB 0.00 kB Deleted 16.14 kB 0.00 kB

Generated by 🚫 dangerJS against c1f1de3

@eps1lon eps1lon force-pushed the rules-of-hooks-eslint-v9 branch 2 times, most recently from 43a0927 to 1a217e9 Compare April 6, 2024 16:59
@eps1lon eps1lon marked this pull request as ready for review April 7, 2024 07:50
return context.sourceCode.getScope(node);
}
}

const scopeManager = context.getSourceCode().scopeManager;
Copy link

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;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSourceCode was only deprecated.

yarn.lock Outdated
Copy link
Member

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?

Copy link
Collaborator Author

@eps1lon eps1lon Apr 22, 2024

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.

Copy link
Member

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

…iveDeps-test.js

Co-authored-by: lauren <poteto@users.noreply.github.com>
@eps1lon eps1lon mentioned this pull request Apr 22, 2024
@eps1lon eps1lon merged commit 6f18664 into facebook:main Apr 23, 2024
38 checks passed
@silkfire
Copy link

When will this be released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants