Skip to content

Commit

Permalink
[compiler] Inferred deps must match exact optionality of manual deps
Browse files Browse the repository at this point in the history
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-source-id: afd49e89cc3194eb3c317ca7434d3fa948896bff
Pull Request resolved: #30840
  • Loading branch information
josephsavona committed Aug 28, 2024
1 parent 3a45ba2 commit fc0df47
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ export function dropManualMemoization(func: HIRFunction): void {
function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
const optionals = new Set<IdentifierId>();
for (const [, block] of fn.body.blocks) {
if (block.terminal.kind === 'optional') {
if (block.terminal.kind === 'optional' && block.terminal.optional) {
const optionalTerminal = block.terminal;
let testBlock = fn.body.blocks.get(block.terminal.test)!;
loop: while (true) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ function compareDeps(
if (inferred.path[i].property !== source.path[i].property) {
isSubpath = false;
break;
} else if (inferred.path[i].optional && !source.path[i].optional) {
} else if (inferred.path[i].optional !== source.path[i].optional) {
/**
* The inferred path must be at least as precise as the manual path:
* if the inferred path is optional, then the source path must have
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@

## Input

```javascript
// @validatePreserveExistingMemoizationGuarantees
function Component(props) {
const data = useMemo(() => {
// actual code is non-optional
return props.items.edges.nodes ?? [];
// deps are optional
}, [props.items?.edges?.nodes]);
return <Foo data={data} />;
}

```
## Error
```
1 | // @validatePreserveExistingMemoizationGuarantees
2 | function Component(props) {
> 3 | const data = useMemo(() => {
| ^^^^^^^
> 4 | // actual code is non-optional
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 5 | return props.items.edges.nodes ?? [];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 6 | // deps are optional
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 7 | }, [props.items?.edges?.nodes]);
| ^^^^ 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)
8 | return <Foo data={data} />;
9 | }
10 |
```

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ function Component(props) {
x.push(props?.items);
x.push(props.items);
return x;
}, [props?.items]);
return <ValidateMemoization inputs={[props?.items]} output={data} />;
}, [props.items]);
return <ValidateMemoization inputs={[props.items]} output={data} />;
}

```
Expand All @@ -23,8 +23,6 @@ import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMe
import { ValidateMemoization } from "shared-runtime";
function Component(props) {
const $ = _c(7);

props?.items;
let t0;
let x;
if ($[0] !== props.items) {
Expand All @@ -38,25 +36,24 @@ function Component(props) {
}
t0 = x;
const data = t0;
const t1 = props?.items;
let t2;
if ($[2] !== t1) {
t2 = [t1];
$[2] = t1;
$[3] = t2;
let t1;
if ($[2] !== props.items) {
t1 = [props.items];
$[2] = props.items;
$[3] = t1;
} else {
t2 = $[3];
t1 = $[3];
}
let t3;
if ($[4] !== t2 || $[5] !== data) {
t3 = <ValidateMemoization inputs={t2} output={data} />;
$[4] = t2;
let t2;
if ($[4] !== t1 || $[5] !== data) {
t2 = <ValidateMemoization inputs={t1} output={data} />;
$[4] = t1;
$[5] = data;
$[6] = t3;
$[6] = t2;
} else {
t3 = $[6];
t2 = $[6];
}
return t3;
return t2;
}

```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ function Component(props) {
x.push(props?.items);
x.push(props.items);
return x;
}, [props?.items]);
return <ValidateMemoization inputs={[props?.items]} output={data} />;
}, [props.items]);
return <ValidateMemoization inputs={[props.items]} output={data} />;
}

0 comments on commit fc0df47

Please sign in to comment.