Compare commits

...

1 Commits

Author SHA1 Message Date
Joe Savona
f5eef1b455 [compiler] Allow manual dependencies to have different optionality than inferred deps
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.
2025-11-24 12:15:54 -08:00
3 changed files with 44 additions and 39 deletions

View File

@@ -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);

View File

@@ -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 | }, []);
```

View File

@@ -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]} />;
}