Skip to content

Commit 0691f5f

Browse files
committed
[compiler] Inferred deps must match exact optionality of manual deps
To prevent any difference in behavior, we check that the optionality of the inferred deps exactly matches the optionality of the manual dependencies. This required a fix, I was incorrectly inferring optionality of manual deps (they're only optional if OptionalTerminal.optional is true) - for nested cases of mixed optional/non-optional. [ghstack-poisoned]
1 parent 51d4844 commit 0691f5f

7 files changed

+57
-72
lines changed

compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ export function dropManualMemoization(func: HIRFunction): void {
488488
function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
489489
const optionals = new Set<IdentifierId>();
490490
for (const [, block] of fn.body.blocks) {
491-
if (block.terminal.kind === 'optional') {
491+
if (block.terminal.kind === 'optional' && block.terminal.optional) {
492492
const optionalTerminal = block.terminal;
493493
let testBlock = fn.body.blocks.get(block.terminal.test)!;
494494
loop: while (true) {

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ function compareDeps(
170170
if (inferred.path[i].property !== source.path[i].property) {
171171
isSubpath = false;
172172
break;
173-
} else if (inferred.path[i].optional && !source.path[i].optional) {
173+
} else if (inferred.path[i].optional !== source.path[i].optional) {
174174
/**
175175
* The inferred path must be at least as precise as the manual path:
176176
* if the inferred path is optional, then the source path must have
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validatePreserveExistingMemoizationGuarantees
6+
function Component(props) {
7+
const data = useMemo(() => {
8+
// actual code is non-optional
9+
return props.items.edges.nodes ?? [];
10+
// deps are optional
11+
}, [props.items?.edges?.nodes]);
12+
return <Foo data={data} />;
13+
}
14+
15+
```
16+
17+
18+
## Error
19+
20+
```
21+
1 | // @validatePreserveExistingMemoizationGuarantees
22+
2 | function Component(props) {
23+
> 3 | const data = useMemo(() => {
24+
| ^^^^^^^
25+
> 4 | // actual code is non-optional
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
27+
> 5 | return props.items.edges.nodes ?? [];
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
> 6 | // deps are optional
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
31+
> 7 | }, [props.items?.edges?.nodes]);
32+
| ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (3:7)
33+
8 | return <Foo data={data} />;
34+
9 | }
35+
10 |
36+
```
37+
38+

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-as-memo-dep-non-optional-in-body.expect.md

Lines changed: 0 additions & 50 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-single-with-unconditional.expect.md

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ function Component(props) {
1010
x.push(props?.items);
1111
x.push(props.items);
1212
return x;
13-
}, [props?.items]);
14-
return <ValidateMemoization inputs={[props?.items]} output={data} />;
13+
}, [props.items]);
14+
return <ValidateMemoization inputs={[props.items]} output={data} />;
1515
}
1616

1717
```
@@ -23,8 +23,6 @@ import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMe
2323
import { ValidateMemoization } from "shared-runtime";
2424
function Component(props) {
2525
const $ = _c(7);
26-
27-
props?.items;
2826
let t0;
2927
let x;
3028
if ($[0] !== props.items) {
@@ -38,25 +36,24 @@ function Component(props) {
3836
}
3937
t0 = x;
4038
const data = t0;
41-
const t1 = props?.items;
42-
let t2;
43-
if ($[2] !== t1) {
44-
t2 = [t1];
45-
$[2] = t1;
46-
$[3] = t2;
39+
let t1;
40+
if ($[2] !== props.items) {
41+
t1 = [props.items];
42+
$[2] = props.items;
43+
$[3] = t1;
4744
} else {
48-
t2 = $[3];
45+
t1 = $[3];
4946
}
50-
let t3;
51-
if ($[4] !== t2 || $[5] !== data) {
52-
t3 = <ValidateMemoization inputs={t2} output={data} />;
53-
$[4] = t2;
47+
let t2;
48+
if ($[4] !== t1 || $[5] !== data) {
49+
t2 = <ValidateMemoization inputs={t1} output={data} />;
50+
$[4] = t1;
5451
$[5] = data;
55-
$[6] = t3;
52+
$[6] = t2;
5653
} else {
57-
t3 = $[6];
54+
t2 = $[6];
5855
}
59-
return t3;
56+
return t2;
6057
}
6158

6259
```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-member-expression-single-with-unconditional.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ function Component(props) {
66
x.push(props?.items);
77
x.push(props.items);
88
return x;
9-
}, [props?.items]);
10-
return <ValidateMemoization inputs={[props?.items]} output={data} />;
9+
}, [props.items]);
10+
return <ValidateMemoization inputs={[props.items]} output={data} />;
1111
}

0 commit comments

Comments
 (0)