Skip to content

Conversation

@jackpope
Copy link
Contributor

react-hooks/exhaustive-effect-dependencies from ValidateExhaustiveDeps reports errors for both missing and extra effect deps. We already have react-hooks/exhaustive-deps that errors on missing dependencies. In the future we'd like to consolidate this all to the compiler based error, but for now there's a lot of overlap. Let's enable testing the extra dep warning by splitting out reporting modes.

This PR

  • Creates on, off, missing-only, and extra-only reporting modes for the effect dep validation flag
  • Temporarily enables the new rule with extra-only in eslint-plugin-react-hooks
  • Adds additional null checking to manualMemoLoc to fix a bug found when running against the fixture

@jackpope jackpope requested a review from poteto December 15, 2025 23:18
@meta-cla meta-cla bot added the CLA Signed label Dec 15, 2025
@jackpope jackpope requested a review from mofeiZ December 15, 2025 23:18
description:
'Validates that effect dependencies are exhaustive and without extraneous values',
preset: LintRulePreset.Off,
preset: LintRulePreset.Recommended,
Copy link
Member

Choose a reason for hiding this comment

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

technically this shouldn't be needed as we don't use presets internally, but okay to leave in if we plan to ship.

if you choose to leave it, you’ll need to manually add the rule to the eslint config for this fixture

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only changed this because the rule noops in the eslint-v9 fixture with it off. But if we can control it separately internally I can disable the fixture test while we experiment.

@react-sizebot
Copy link

Comparing: bcf97c7...61f0cb3

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.84 kB 6.84 kB = 1.88 kB 1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 608.16 kB 608.16 kB = 107.67 kB 107.67 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.84 kB 6.84 kB +0.05% 1.88 kB 1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 667.39 kB 667.39 kB = 117.55 kB 117.55 kB
facebook-www/ReactDOM-prod.classic.js = 693.47 kB 693.47 kB = 122.05 kB 122.05 kB
facebook-www/ReactDOM-prod.modern.js = 683.89 kB 683.89 kB = 120.43 kB 120.43 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 61f0cb3

@jackpope jackpope merged commit 88ee1f5 into facebook:main Dec 15, 2025
251 checks passed
@jackpope jackpope deleted the extra-only-effect-dep-validation branch December 15, 2025 23:59
github-actions bot pushed a commit that referenced this pull request Dec 16, 2025
…d temporarily enable (#35365)

`react-hooks/exhaustive-effect-dependencies` from
`ValidateExhaustiveDeps` reports errors for both missing and extra
effect deps. We already have `react-hooks/exhaustive-deps` that errors
on missing dependencies. In the future we'd like to consolidate this all
to the compiler based error, but for now there's a lot of overlap. Let's
enable testing the extra dep warning by splitting out reporting modes.

This PR
- Creates `on`, `off`, `missing-only`, and `extra-only` reporting modes
for the effect dep validation flag
- Temporarily enables the new rule with `extra-only` in
`eslint-plugin-react-hooks`
- Adds additional null checking to `manualMemoLoc` to fix a bug found
when running against the fixture

DiffTrain build for [88ee1f5](88ee1f5)
github-actions bot pushed a commit that referenced this pull request Dec 16, 2025
…d temporarily enable (#35365)

`react-hooks/exhaustive-effect-dependencies` from
`ValidateExhaustiveDeps` reports errors for both missing and extra
effect deps. We already have `react-hooks/exhaustive-deps` that errors
on missing dependencies. In the future we'd like to consolidate this all
to the compiler based error, but for now there's a lot of overlap. Let's
enable testing the extra dep warning by splitting out reporting modes.

This PR
- Creates `on`, `off`, `missing-only`, and `extra-only` reporting modes
for the effect dep validation flag
- Temporarily enables the new rule with `extra-only` in
`eslint-plugin-react-hooks`
- Adds additional null checking to `manualMemoLoc` to fix a bug found
when running against the fixture

DiffTrain build for [88ee1f5](88ee1f5)
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.

3 participants