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

refactor(eslint-plugin-react-hooks): change array type and improve conditionals #32400

Merged

Conversation

michaelfaith
Copy link
Contributor

@michaelfaith michaelfaith commented Feb 16, 2025

This change adds configuration to the eslint config governing `eslint-plugin-react-hooks` to use the typescript-eslint plugin and parser.  It adds the typescript-recommended config, and configures the team's preferred `array-type` convention.
This change addresses several feedback items from facebook#32240
@react-sizebot
Copy link

react-sizebot commented Feb 16, 2025

Comparing: eb1f77d...9913a7b

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 = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 515.71 kB 515.71 kB = 92.09 kB 92.09 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 562.25 kB 562.25 kB = 100.08 kB 100.08 kB
facebook-www/ReactDOM-prod.classic.js = 636.70 kB 636.70 kB = 112.08 kB 112.08 kB
facebook-www/ReactDOM-prod.modern.js = 627.02 kB 627.02 kB = 110.49 kB 110.49 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 6ea4c0d

@michaelfaith michaelfaith marked this pull request as ready for review February 16, 2025 23:19
@@ -40,11 +40,11 @@ jobs:
uses: actions/cache@v4
id: node_modules
with:
path: "**/node_modules"
path: "node_modules"
Copy link
Member

Choose a reason for hiding this comment

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

Was this just to bust the cache? I don't see why what it was previously needs to be changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The node_modules within the fixtures are soft-linking to the build directory for the eslint-plugin. When those node_modules were fulfilled from cache, the symlinks were broken, and after the build, it's not re-linked. So this changes it so that it's still caching the root node_modules, but not the nested ones. That way those node_modules with the symlinks can be created each time.

This change removes the nested fixture node_modules from being cached, so that the symbolic link can be made after the build happens.

ci (eslint-e2e): exclude nested `node_modules` from cache

This change removes the nested fixture node_modules from being cached, so that the symbolic link can be made after the build happens.
@michaelfaith michaelfaith force-pushed the build/add-typescript-eslint-to-eslint-plugin branch from ff47b9e to 6ea4c0d Compare February 17, 2025 00:26
@poteto poteto merged commit 4632e36 into facebook:main Feb 17, 2025
194 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 17, 2025
…nditionals (#32400)

- [build(eslint-plugin-react-hooks): add
ts-linting](4c0fbe7)
This change adds configuration to the eslint config governing
`eslint-plugin-react-hooks` to use the typescript-eslint plugin and
parser. It adds the typescript-recommended config, and configures the
team's preferred `array-type` convention.

- [refactor(eslint-plugin-react-hooks): improve
conditionals](540d0d9)
This change addresses several feedback items from
#32240

- [ci (eslint-e2e): exclude nested node_modules from
cache](a3279f4)
This change removes the nested fixture `node_modules` from being cached,
so that the symbolic link can be made after the build happens.

DiffTrain build for [4632e36](4632e36)
@michaelfaith michaelfaith deleted the build/add-typescript-eslint-to-eslint-plugin branch February 17, 2025 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants