Skip to content

Commit

Permalink
[compiler] Allow inferred non-optional paths when manual deps were op…
Browse files Browse the repository at this point in the history
…tional

If the inferred deps are more precise (non-optional) than the manual deps (optional) it should pass validation.

The other direction also seems like it would be fine - inferring optional deps when the original was non-optional - but for now let's keep the "at least as precise" rule.

ghstack-source-id: 8f34860deafefe48c7a374caa1b55482e571308d
Pull Request resolved: #30816
  • Loading branch information
josephsavona committed Aug 27, 2024
1 parent 76ce3ed commit ac01cf7
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,16 @@ function compareDeps(

let isSubpath = true;
for (let i = 0; i < Math.min(inferred.path.length, source.path.length); i++) {
if (
inferred.path[i].property !== source.path[i].property ||
inferred.path[i].optional !== source.path[i].optional
) {
if (inferred.path[i].property !== source.path[i].property) {
isSubpath = false;
break;
} 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
* been optional too.
*/
return CompareDependencyResult.PathDifference;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@

## 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} />;
}

```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees
function Component(props) {
const $ = _c(4);

props.items?.edges?.nodes;
let t0;
let t1;
if ($[0] !== props.items.edges.nodes) {
t1 = props.items.edges.nodes ?? [];
$[0] = props.items.edges.nodes;
$[1] = t1;
} else {
t1 = $[1];
}
t0 = t1;
const data = t0;
let t2;
if ($[2] !== data) {
t2 = <Foo data={data} />;
$[2] = data;
$[3] = t2;
} else {
t2 = $[3];
}
return t2;
}

```
### Eval output
(kind: exception) Fixture not implemented
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// @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} />;
}

0 comments on commit ac01cf7

Please sign in to comment.