Skip to content

Commit 4d59820

Browse files
committed
[compiler] Ignore ESLint suppressions when ValidateMemoDeps enabled
With `ValidateExhaustiveMemoDependencies` we can now check exhaustive dependencies for useMemo and useCallback within the compiler, without relying on the separate exhaustive-deps rule. Until now we've bailed out of any component/hook that suppresses this rule, since the suppression _might_ affect a memoization value. Compiling code with incorrect memo deps can change behavior so this wasn't safe. The downside was that a suppression within a useEffect could prevent memoization, even though non-exhaustive deps for effects do not cause problems for memoization specifically. So here, we change to ignore ESLint suppressions if we have both the compiler's hooks validation and memo deps validations enabled. Now we just have to test out the new validation and refine before we can enable this by default.
1 parent 71cd580 commit 4d59820

File tree

5 files changed

+126
-3
lines changed

5 files changed

+126
-3
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,15 @@ export function compileProgram(
400400
*/
401401
const suppressions = findProgramSuppressions(
402402
pass.comments,
403-
pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS,
403+
/*
404+
* If the compiler is validating hooks rules and exhaustive memo dependencies, we don't need to check
405+
* for React ESLint suppressions
406+
*/
407+
pass.opts.environment.validateExhaustiveMemoizationDependencies &&
408+
pass.opts.environment.validateHooksUsage
409+
? null
410+
: (pass.opts.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS),
411+
// Always bail on Flow suppressions
404412
pass.opts.flowSuppressions,
405413
);
406414

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Suppression.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export function filterSuppressionsThatAffectFunction(
7878

7979
export function findProgramSuppressions(
8080
programComments: Array<t.Comment>,
81-
ruleNames: Array<string>,
81+
ruleNames: Array<string> | null,
8282
flowSuppressions: boolean,
8383
): Array<SuppressionRange> {
8484
const suppressionRanges: Array<SuppressionRange> = [];
@@ -89,7 +89,7 @@ export function findProgramSuppressions(
8989
let disableNextLinePattern: RegExp | null = null;
9090
let disablePattern: RegExp | null = null;
9191
let enablePattern: RegExp | null = null;
92-
if (ruleNames.length !== 0) {
92+
if (ruleNames != null && ruleNames.length !== 0) {
9393
const rulePattern = `(${ruleNames.join('|')})`;
9494
disableNextLinePattern = new RegExp(
9595
`eslint-disable-next-line ${rulePattern}`,
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateExhaustiveMemoizationDependencies
6+
7+
import {useMemo} from 'react';
8+
import {ValidateMemoization} from 'shared-runtime';
9+
10+
function Component({x}) {
11+
useEffect(
12+
() => {
13+
console.log(x);
14+
// eslint-disable-next-line react-hooks/exhaustive-deps
15+
},
16+
[
17+
/* intentionally missing deps */
18+
]
19+
);
20+
21+
const memo = useMemo(() => {
22+
return [x];
23+
}, [x]);
24+
25+
return <ValidateMemoization inputs={[x]} output={memo} />;
26+
}
27+
28+
```
29+
30+
## Code
31+
32+
```javascript
33+
import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
34+
35+
import { useMemo } from "react";
36+
import { ValidateMemoization } from "shared-runtime";
37+
38+
function Component(t0) {
39+
const $ = _c(10);
40+
const { x } = t0;
41+
let t1;
42+
if ($[0] !== x) {
43+
t1 = () => {
44+
console.log(x);
45+
};
46+
$[0] = x;
47+
$[1] = t1;
48+
} else {
49+
t1 = $[1];
50+
}
51+
let t2;
52+
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
53+
t2 = [];
54+
$[2] = t2;
55+
} else {
56+
t2 = $[2];
57+
}
58+
useEffect(t1, t2);
59+
let t3;
60+
if ($[3] !== x) {
61+
t3 = [x];
62+
$[3] = x;
63+
$[4] = t3;
64+
} else {
65+
t3 = $[4];
66+
}
67+
const memo = t3;
68+
let t4;
69+
if ($[5] !== x) {
70+
t4 = [x];
71+
$[5] = x;
72+
$[6] = t4;
73+
} else {
74+
t4 = $[6];
75+
}
76+
let t5;
77+
if ($[7] !== memo || $[8] !== t4) {
78+
t5 = <ValidateMemoization inputs={t4} output={memo} />;
79+
$[7] = memo;
80+
$[8] = t4;
81+
$[9] = t5;
82+
} else {
83+
t5 = $[9];
84+
}
85+
return t5;
86+
}
87+
88+
```
89+
90+
### Eval output
91+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// @validateExhaustiveMemoizationDependencies
2+
3+
import {useMemo} from 'react';
4+
import {ValidateMemoization} from 'shared-runtime';
5+
6+
function Component({x}) {
7+
useEffect(
8+
() => {
9+
console.log(x);
10+
// eslint-disable-next-line react-hooks/exhaustive-deps
11+
},
12+
[
13+
/* intentionally missing deps */
14+
]
15+
);
16+
17+
const memo = useMemo(() => {
18+
return [x];
19+
}, [x]);
20+
21+
return <ValidateMemoization inputs={[x]} output={memo} />;
22+
}

compiler/packages/snap/src/runner.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,10 @@ const opts: RunnerOptions = yargs
5353
.default('worker-threads', true)
5454
.boolean('watch')
5555
.describe('watch', 'Run compiler in watch mode, re-running after changes')
56+
.alias('w', 'watch')
5657
.default('watch', false)
5758
.boolean('update')
59+
.alias('u', 'update')
5860
.describe('update', 'Update fixtures')
5961
.default('update', false)
6062
.boolean('filter')

0 commit comments

Comments
 (0)