amirho/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2026-02-21 19:31:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add reporting modes for react-hooks/exhaustive-effect-dependencies and temporarily enable (#35365)

Browse Source 
`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
This commit is contained in:
Jack Pope
2025-12-15 18:59:27 -05:00
committed by GitHub
parent bcf97c7564
commit 88ee1f5955
17 changed files with 293 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
View File

@@ -225,8 +225,15 @@ export const EnvironmentConfigSchema = z.object({
/**
* Validate that dependencies supplied to effect hooks are exhaustive.
* Can be:
* - 'off': No validation (default)
* - 'all': Validate and report both missing and extra dependencies
* - 'missing-only': Only report missing dependencies
* - 'extra-only': Only report extra/unnecessary dependencies
*/
validateExhaustiveEffectDependencies: z.boolean().default(false),
validateExhaustiveEffectDependencies: z
.enum(['off', 'all', 'missing-only', 'extra-only'])
.default('off'),
/**
* When this is true, rather than pruning existing manual memoization but ensuring or validating

35
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts
View File

@@ -141,6 +141,7 @@ export function validateExhaustiveDependencies(
reactive,
startMemo.depsLoc,
ErrorCategory.MemoDependencies,
'all',
);
if (diagnostic != null) {
error.pushDiagnostic(diagnostic);
@@ -159,7 +160,7 @@ export function validateExhaustiveDependencies(
onStartMemoize,
onFinishMemoize,
onEffect: (inferred, manual, manualMemoLoc) => {
if (env.config.validateExhaustiveEffectDependencies === false) {
if (env.config.validateExhaustiveEffectDependencies === 'off') {
return;
}
if (DEBUG) {
@@ -195,12 +196,17 @@ export function validateExhaustiveDependencies(
});
}
}
const effectReportMode =
typeof env.config.validateExhaustiveEffectDependencies === 'string'
? env.config.validateExhaustiveEffectDependencies
: 'all';
const diagnostic = validateDependencies(
Array.from(inferred),
manualDeps,
reactive,
manualMemoLoc,
ErrorCategory.EffectExhaustiveDependencies,
effectReportMode,
);
if (diagnostic != null) {
error.pushDiagnostic(diagnostic);
@@ -220,6 +226,7 @@ function validateDependencies(
category:
| ErrorCategory.MemoDependencies
| ErrorCategory.EffectExhaustiveDependencies,
exhaustiveDepsReportMode: 'all' | 'missing-only' | 'extra-only',
): CompilerDiagnostic | null {
// Sort dependencies by name and path, with shorter/non-optional paths first
inferred.sort((a, b) => {
@@ -370,9 +377,20 @@ function validateDependencies(
extra.push(dep);
}
if (missing.length !== 0 || extra.length !== 0) {
// Filter based on report mode
const filteredMissing =
exhaustiveDepsReportMode === 'extra-only' ? [] : missing;
const filteredExtra =
exhaustiveDepsReportMode === 'missing-only' ? [] : extra;
if (filteredMissing.length !== 0 || filteredExtra.length !== 0) {
let suggestion: CompilerSuggestion | null = null;
if (manualMemoLoc != null && typeof manualMemoLoc !== 'symbol') {
if (
manualMemoLoc != null &&
typeof manualMemoLoc !== 'symbol' &&
manualMemoLoc.start.index != null &&
manualMemoLoc.end.index != null
) {
suggestion = {
description: 'Update dependencies',
range: [manualMemoLoc.start.index, manualMemoLoc.end.index],
@@ -388,8 +406,13 @@ function validateDependencies(
.join(', ')}]`,
};
}
const diagnostic = createDiagnostic(category, missing, extra, suggestion);
for (const dep of missing) {
const diagnostic = createDiagnostic(
category,
filteredMissing,
filteredExtra,
suggestion,
);
for (const dep of filteredMissing) {
let reactiveStableValueHint = '';
if (isStableType(dep.identifier)) {
reactiveStableValueHint =
@@ -402,7 +425,7 @@ function validateDependencies(
loc: dep.loc,
});
}
for (const dep of extra) {
for (const dep of filteredExtra) {
if (dep.root.kind === 'Global') {
diagnostic.withDetails({
kind: 'error',

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md
View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateExhaustiveEffectDependencies
// @validateExhaustiveEffectDependencies:"all"
import {useEffect, useEffectEvent} from 'react';
function Component({x, y, z}) {

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.js
View File

@@ -1,4 +1,4 @@
// @validateExhaustiveEffectDependencies
// @validateExhaustiveEffectDependencies:"all"
import {useEffect, useEffectEvent} from 'react';
function Component({x, y, z}) {

83
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps-extra-only.expect.md Normal file
View File

@@ -0,0 +1,83 @@
## Input
```javascript
// @validateExhaustiveEffectDependencies:"extra-only"
import {useEffect} from 'react';
function Component({x, y, z}) {
// no error: missing dep not reported in extra-only mode
useEffect(() => {
log(x);
}, []);
// error: extra dep - y
useEffect(() => {
log(x);
}, [x, y]);
// error: extra dep - y (missing dep - z not reported)
useEffect(() => {
log(x, z);
}, [x, y]);
// error: extra dep - x.y
useEffect(() => {
log(x);
}, [x.y]);
}
```
## Error
```
Found 3 errors:
Error: Found extra effect dependencies
Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects.
error.invalid-exhaustive-effect-deps-extra-only.ts:13:9
11 | useEffect(() => {
12 | log(x);
> 13 | }, [x, y]);
| ^ Unnecessary dependency `y`
14 |
15 | // error: extra dep - y (missing dep - z not reported)
16 | useEffect(() => {
Inferred dependencies: `[x]`
Error: Found extra effect dependencies
Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects.
error.invalid-exhaustive-effect-deps-extra-only.ts:18:9
16 | useEffect(() => {
17 | log(x, z);
> 18 | }, [x, y]);
| ^ Unnecessary dependency `y`
19 |
20 | // error: extra dep - x.y
21 | useEffect(() => {
Inferred dependencies: `[x, z]`
Error: Found extra effect dependencies
Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects.
error.invalid-exhaustive-effect-deps-extra-only.ts:23:6
21 | useEffect(() => {
22 | log(x);
> 23 | }, [x.y]);
| ^^^ Overly precise dependency `x.y`, use `x` instead
24 | }
25 |
Inferred dependencies: `[x]`
```

24
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps-extra-only.js Normal file
View File

@@ -0,0 +1,24 @@
// @validateExhaustiveEffectDependencies:"extra-only"
import {useEffect} from 'react';
function Component({x, y, z}) {
// no error: missing dep not reported in extra-only mode
useEffect(() => {
log(x);
}, []);
// error: extra dep - y
useEffect(() => {
log(x);
}, [x, y]);
// error: extra dep - y (missing dep - z not reported)
useEffect(() => {
log(x, z);
}, [x, y]);
// error: extra dep - x.y
useEffect(() => {
log(x);
}, [x.y]);
}

84
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps-missing-only.expect.md Normal file
View File

@@ -0,0 +1,84 @@
## Input
```javascript
// @validateExhaustiveEffectDependencies:"missing-only"
import {useEffect} from 'react';
function Component({x, y, z}) {
// error: missing dep - x
useEffect(() => {
log(x);
}, []);
// no error: extra dep not reported in missing-only mode
useEffect(() => {
log(x);
}, [x, y]);
// error: missing dep - z (extra dep - y not reported)
useEffect(() => {
log(x, z);
}, [x, y]);
// error: missing dep x
useEffect(() => {
log(x);
}, [x.y]);
}
```
## Error
```
Found 3 errors:
Error: Found missing effect dependencies
Missing dependencies can cause an effect to fire less often than it should.
error.invalid-exhaustive-effect-deps-missing-only.ts:7:8
5 | // error: missing dep - x
6 | useEffect(() => {
> 7 | log(x);
| ^ Missing dependency `x`
8 | }, []);
9 |
10 | // no error: extra dep not reported in missing-only mode
Inferred dependencies: `[x]`
Error: Found missing effect dependencies
Missing dependencies can cause an effect to fire less often than it should.
error.invalid-exhaustive-effect-deps-missing-only.ts:17:11
15 | // error: missing dep - z (extra dep - y not reported)
16 | useEffect(() => {
> 17 | log(x, z);
| ^ Missing dependency `z`
18 | }, [x, y]);
19 |
20 | // error: missing dep x
Inferred dependencies: `[x, z]`
Error: Found missing effect dependencies
Missing dependencies can cause an effect to fire less often than it should.
error.invalid-exhaustive-effect-deps-missing-only.ts:22:8
20 | // error: missing dep x
21 | useEffect(() => {
> 22 | log(x);
| ^ Missing dependency `x`
23 | }, [x.y]);
24 | }
25 |
Inferred dependencies: `[x]`
```

24
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps-missing-only.js Normal file
View File

@@ -0,0 +1,24 @@
// @validateExhaustiveEffectDependencies:"missing-only"
import {useEffect} from 'react';
function Component({x, y, z}) {
// error: missing dep - x
useEffect(() => {
log(x);
}, []);
// no error: extra dep not reported in missing-only mode
useEffect(() => {
log(x);
}, [x, y]);
// error: missing dep - z (extra dep - y not reported)
useEffect(() => {
log(x, z);
}, [x, y]);
// error: missing dep x
useEffect(() => {
log(x);
}, [x.y]);
}

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md
View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateExhaustiveEffectDependencies
// @validateExhaustiveEffectDependencies:"all"
import {useEffect} from 'react';
function Component({x, y, z}) {

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js
View File

@@ -1,4 +1,4 @@
// @validateExhaustiveEffectDependencies
// @validateExhaustiveEffectDependencies:"all"
import {useEffect} from 'react';
function Component({x, y, z}) {

4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md
View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies
// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies:"all"
import {
useCallback,
useTransition,
@@ -69,7 +69,7 @@ export const FIXTURE_ENTRYPOINT = {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies
import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies:"all"
import {
useCallback,
useTransition,

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js
View File

@@ -1,4 +1,4 @@
// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies
// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies:"all"
import {
useCallback,
useTransition,

4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.expect.md
View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateExhaustiveEffectDependencies
// @validateExhaustiveEffectDependencies:"all"
import {useEffect, useEffectEvent} from 'react';
function Component({x, y, z}) {
@@ -30,7 +30,7 @@ function Component({x, y, z}) {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveEffectDependencies
import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveEffectDependencies:"all"
import { useEffect, useEffectEvent } from "react";
function Component(t0) {

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.js
View File

@@ -1,4 +1,4 @@
// @validateExhaustiveEffectDependencies
// @validateExhaustiveEffectDependencies:"all"
import {useEffect, useEffectEvent} from 'react';
function Component({x, y, z}) {

13
fixtures/eslint-v9/index.js
View File

@@ -167,3 +167,16 @@ function InvalidUseMemo({items}) {
const sorted = useMemo(() => [...items].sort(), []);
return <div>{sorted.length}</div>;
}
// Invalid: missing/extra deps in useEffect
function InvalidEffectDeps({a, b}) {
useEffect(() => {
console.log(a);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
useEffect(() => {
console.log(a);
// TODO: eslint-disable-next-line react-hooks/exhaustive-effect-dependencies
}, [a, b]);
}

22
fixtures/eslint-v9/yarn.lock
View File

@@ -603,6 +603,18 @@ has-flag@^4.0.0:
resolved "https://registry.yarnpkg.com/has-flag/-/has-flag-4.0.0.tgz#944771fd9c81c81265c4d6941860da06bb59479b"
integrity sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==
hermes-estree@0.25.1:
version "0.25.1"
resolved "https://registry.yarnpkg.com/hermes-estree/-/hermes-estree-0.25.1.tgz#6aeec17d1983b4eabf69721f3aa3eb705b17f480"
integrity sha512-0wUoCcLp+5Ev5pDW2OriHC2MJCbwLwuRx+gAqMTOkGKJJiBCLjtrvy4PWUGn6MIVefecRpzoOZ/UV6iGdOr+Cw==
hermes-parser@^0.25.1:
version "0.25.1"
resolved "https://registry.yarnpkg.com/hermes-parser/-/hermes-parser-0.25.1.tgz#5be0e487b2090886c62bd8a11724cd766d5f54d1"
integrity sha512-6pEjquH3rqaI6cYAXYPcz9MS4rY6R4ngRgrgfDshRptUZIc3lw0MCIJIGDj9++mfySOuPTHB4nrSW99BCvOPIA==
dependencies:
hermes-estree "0.25.1"
ignore@^5.2.0:
version "5.3.2"
resolved "https://registry.yarnpkg.com/ignore/-/ignore-5.3.2.tgz#3cd40e729f3643fd87cb04e50bf0eb722bc596f5"
@@ -877,12 +889,12 @@ yocto-queue@^0.1.0:
resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b"
integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==
"zod-validation-error@^3.0.3 || ^4.0.0":
"zod-validation-error@^3.5.0 || ^4.0.0":
version "4.0.2"
resolved "https://registry.yarnpkg.com/zod-validation-error/-/zod-validation-error-4.0.2.tgz#bc605eba49ce0fcd598c127fee1c236be3f22918"
integrity sha512-Q6/nZLe6jxuU80qb/4uJ4t5v2VEZ44lzQjPDhYJNztRQ4wyWc6VF3D3Kb/fAuPetZQnhS3hnajCf9CsWesghLQ==
"zod@^3.22.4 || ^4.0.0":
version "4.1.11"
resolved "https://registry.yarnpkg.com/zod/-/zod-4.1.11.tgz#4aab62f76cfd45e6c6166519ba31b2ea019f75f5"
integrity sha512-WPsqwxITS2tzx1bzhIKsEs19ABD5vmCVa4xBo2tq/SrV4RNZtfws1EnCWQXM6yh8bD08a1idvkB5MZSBiZsjwg==
"zod@^3.25.0 || ^4.0.0":
version "4.2.0"
resolved "https://registry.yarnpkg.com/zod/-/zod-4.2.0.tgz#01e86f2c2b6d525a1b9fa6dbe78beccad082118f"
integrity sha512-Bd5fw9wlIhtqCCxotZgdTOMwGm1a0u75wARVEY9HMs1X17trvA/lMi4+MGK5EUfYkXVTbX8UDiDKW4OgzHVUZw==

1
packages/eslint-plugin-react-hooks/src/shared/RunReactCompiler.ts
View File

@@ -42,6 +42,7 @@ const COMPILER_OPTIONS: PluginOptions = {
// Temporarily enabled for internal testing
enableUseKeyedState: true,
enableVerboseNoSetStateInEffect: true,
validateExhaustiveEffectDependencies: 'extra-only',
},
};