mirror of
https://github.com/zebrajr/react.git
synced 2026-01-15 12:15:22 +00:00
[compiler] Allow manual dependencies to have different optionality than inferred deps (#35186)
Since adding this validation we've already changed our inference to use knowledge from manual memoization to inform when values are frozen and which values are non-nullable. To align with that, if the user chooses to use different optionality btw the deps and the memo block/callback, that's fine. The key is that eg `x?.y` will invalidate whenever `x.y` would, so from a memoization correctness perspective its fine. It's not our job to be a type checker: if a value is potentially nullable, it should likely use a nullable property access in both places but TypeScript/Flow can check that. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35186). * #35201 * #35202 * #35192 * #35190 * __->__ #35186
This commit is contained in:
@@ -24,6 +24,7 @@ import {
|
||||
InstructionKind,
|
||||
isStableType,
|
||||
isSubPath,
|
||||
isSubPathIgnoringOptionals,
|
||||
isUseRefType,
|
||||
LoadGlobal,
|
||||
ManualMemoDependency,
|
||||
@@ -240,7 +241,10 @@ export function validateExhaustiveDependencies(
|
||||
manualDependency.root.value.identifier.id ===
|
||||
inferredDependency.identifier.id &&
|
||||
(areEqualPaths(manualDependency.path, inferredDependency.path) ||
|
||||
isSubPath(manualDependency.path, inferredDependency.path))
|
||||
isSubPathIgnoringOptionals(
|
||||
manualDependency.path,
|
||||
inferredDependency.path,
|
||||
))
|
||||
) {
|
||||
hasMatchingManualDependency = true;
|
||||
matched.add(manualDependency);
|
||||
|
||||
@@ -9,23 +9,29 @@ import {Stringify} from 'shared-runtime';
|
||||
function Component({x, y, z}) {
|
||||
const a = useMemo(() => {
|
||||
return x?.y.z?.a;
|
||||
// error: too precise
|
||||
}, [x?.y.z?.a.b]);
|
||||
const b = useMemo(() => {
|
||||
return x.y.z?.a;
|
||||
// ok, not our job to type check nullability
|
||||
}, [x.y.z.a]);
|
||||
const c = useMemo(() => {
|
||||
return x?.y.z.a?.b;
|
||||
// error: too precise
|
||||
}, [x?.y.z.a?.b.z]);
|
||||
const d = useMemo(() => {
|
||||
return x?.y?.[(console.log(y), z?.b)];
|
||||
// ok
|
||||
}, [x?.y, y, z?.b]);
|
||||
const e = useMemo(() => {
|
||||
const e = [];
|
||||
e.push(x);
|
||||
return e;
|
||||
// ok
|
||||
}, [x]);
|
||||
const f = useMemo(() => {
|
||||
return [];
|
||||
// error: unnecessary
|
||||
}, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
|
||||
const ref1 = useRef(null);
|
||||
const ref2 = useRef(null);
|
||||
@@ -34,6 +40,7 @@ function Component({x, y, z}) {
|
||||
return () => {
|
||||
return ref.current;
|
||||
};
|
||||
// error: ref is a stable type but reactive
|
||||
}, []);
|
||||
return <Stringify results={[a, b, c, d, e, f, cb]} />;
|
||||
}
|
||||
@@ -44,7 +51,7 @@ function Component({x, y, z}) {
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 5 errors:
|
||||
Found 4 errors:
|
||||
|
||||
Error: Found non-exhaustive dependencies
|
||||
|
||||
@@ -55,61 +62,48 @@ error.invalid-exhaustive-deps.ts:7:11
|
||||
6 | const a = useMemo(() => {
|
||||
> 7 | return x?.y.z?.a;
|
||||
| ^^^^^^^^^ Missing dependency `x?.y.z?.a`
|
||||
8 | }, [x?.y.z?.a.b]);
|
||||
9 | const b = useMemo(() => {
|
||||
10 | return x.y.z?.a;
|
||||
8 | // error: too precise
|
||||
9 | }, [x?.y.z?.a.b]);
|
||||
10 | const b = useMemo(() => {
|
||||
|
||||
Error: Found non-exhaustive dependencies
|
||||
|
||||
Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
|
||||
|
||||
error.invalid-exhaustive-deps.ts:10:11
|
||||
8 | }, [x?.y.z?.a.b]);
|
||||
9 | const b = useMemo(() => {
|
||||
> 10 | return x.y.z?.a;
|
||||
| ^^^^^^^^ Missing dependency `x.y.z?.a`
|
||||
11 | }, [x.y.z.a]);
|
||||
12 | const c = useMemo(() => {
|
||||
13 | return x?.y.z.a?.b;
|
||||
|
||||
Error: Found non-exhaustive dependencies
|
||||
|
||||
Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
|
||||
|
||||
error.invalid-exhaustive-deps.ts:13:11
|
||||
11 | }, [x.y.z.a]);
|
||||
12 | const c = useMemo(() => {
|
||||
> 13 | return x?.y.z.a?.b;
|
||||
error.invalid-exhaustive-deps.ts:15:11
|
||||
13 | }, [x.y.z.a]);
|
||||
14 | const c = useMemo(() => {
|
||||
> 15 | return x?.y.z.a?.b;
|
||||
| ^^^^^^^^^^^ Missing dependency `x?.y.z.a?.b`
|
||||
14 | }, [x?.y.z.a?.b.z]);
|
||||
15 | const d = useMemo(() => {
|
||||
16 | return x?.y?.[(console.log(y), z?.b)];
|
||||
16 | // error: too precise
|
||||
17 | }, [x?.y.z.a?.b.z]);
|
||||
18 | const d = useMemo(() => {
|
||||
|
||||
Error: Found unnecessary memoization dependencies
|
||||
|
||||
Unnecessary dependencies can cause a value to update more often than necessary, which can cause effects to run more than expected.
|
||||
|
||||
error.invalid-exhaustive-deps.ts:25:5
|
||||
23 | const f = useMemo(() => {
|
||||
24 | return [];
|
||||
> 25 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
|
||||
error.invalid-exhaustive-deps.ts:31:5
|
||||
29 | return [];
|
||||
30 | // error: unnecessary
|
||||
> 31 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`, `UNUSED_GLOBAL`
|
||||
26 | const ref1 = useRef(null);
|
||||
27 | const ref2 = useRef(null);
|
||||
28 | const ref = z ? ref1 : ref2;
|
||||
32 | const ref1 = useRef(null);
|
||||
33 | const ref2 = useRef(null);
|
||||
34 | const ref = z ? ref1 : ref2;
|
||||
|
||||
Error: Found non-exhaustive dependencies
|
||||
|
||||
Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI.
|
||||
|
||||
error.invalid-exhaustive-deps.ts:31:13
|
||||
29 | const cb = useMemo(() => {
|
||||
30 | return () => {
|
||||
> 31 | return ref.current;
|
||||
error.invalid-exhaustive-deps.ts:37:13
|
||||
35 | const cb = useMemo(() => {
|
||||
36 | return () => {
|
||||
> 37 | return ref.current;
|
||||
| ^^^ Missing dependency `ref`. Refs, setState functions, and other "stable" values generally do not need to be added as dependencies, but this variable may change over time to point to different values
|
||||
32 | };
|
||||
33 | }, []);
|
||||
34 | return <Stringify results={[a, b, c, d, e, f, cb]} />;
|
||||
38 | };
|
||||
39 | // error: ref is a stable type but reactive
|
||||
40 | }, []);
|
||||
```
|
||||
|
||||
|
||||
@@ -5,23 +5,29 @@ import {Stringify} from 'shared-runtime';
|
||||
function Component({x, y, z}) {
|
||||
const a = useMemo(() => {
|
||||
return x?.y.z?.a;
|
||||
// error: too precise
|
||||
}, [x?.y.z?.a.b]);
|
||||
const b = useMemo(() => {
|
||||
return x.y.z?.a;
|
||||
// ok, not our job to type check nullability
|
||||
}, [x.y.z.a]);
|
||||
const c = useMemo(() => {
|
||||
return x?.y.z.a?.b;
|
||||
// error: too precise
|
||||
}, [x?.y.z.a?.b.z]);
|
||||
const d = useMemo(() => {
|
||||
return x?.y?.[(console.log(y), z?.b)];
|
||||
// ok
|
||||
}, [x?.y, y, z?.b]);
|
||||
const e = useMemo(() => {
|
||||
const e = [];
|
||||
e.push(x);
|
||||
return e;
|
||||
// ok
|
||||
}, [x]);
|
||||
const f = useMemo(() => {
|
||||
return [];
|
||||
// error: unnecessary
|
||||
}, [x, y.z, z?.y?.a, UNUSED_GLOBAL]);
|
||||
const ref1 = useRef(null);
|
||||
const ref2 = useRef(null);
|
||||
@@ -30,6 +36,7 @@ function Component({x, y, z}) {
|
||||
return () => {
|
||||
return ref.current;
|
||||
};
|
||||
// error: ref is a stable type but reactive
|
||||
}, []);
|
||||
return <Stringify results={[a, b, c, d, e, f, cb]} />;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user