Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,32 @@ function collectNonNullsInBlocks(
assumedNonNullObjects.add(entry);
}
}
} else if (
fn.env.config.enablePreserveExistingMemoizationGuarantees &&
instr.value.kind === 'StartMemoize' &&
instr.value.deps != null
) {
for (const dep of instr.value.deps) {
if (dep.root.kind === 'NamedLocal') {
if (
!isImmutableAtInstr(dep.root.value.identifier, instr.id, context)
) {
continue;
}
for (let i = 0; i < dep.path.length; i++) {
const pathEntry = dep.path[i]!;
if (pathEntry.optional) {
break;
}
const depNode = context.registry.getOrCreateProperty({
identifier: dep.root.value.identifier,
path: dep.path.slice(0, i),
reactive: dep.root.value.reactive,
});
assumedNonNullObjects.add(depNode);
}
}
}
}
}

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

## Input

```javascript
// @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees @enableOptionalDependencies @enableTreatFunctionDepsAsConditional:false

import {useMemo} from 'react';
import {identity, ValidateMemoization} from 'shared-runtime';

function Component({x}) {
const object = useMemo(() => {
return identity({
callback: () => {
// This is a bug in our dependency inference: we stop capturing dependencies
// after x.a.b?.c. But what this dependency is telling us is that if `x.a.b`
// was non-nullish, then we can access `.c.d?.e`. Thus we should take the
// full property chain, exactly as-is with optionals/non-optionals, as a
// dependency
return identity(x.a.b?.c.d?.e);
},
});
}, [x.a.b?.c.d?.e]);
const result = useMemo(() => {
return [object.callback()];
}, [object]);
return <Inner x={x} result={result} />;
}

function Inner({x, result}) {
'use no memo';
return <ValidateMemoization inputs={[x.y.z]} output={result} />;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{x: {y: {z: 42}}}],
sequentialRenders: [
{x: {y: {z: 42}}},
{x: {y: {z: 42}}},
{x: {y: {z: 3.14}}},
{x: {y: {z: 42}}},
{x: {y: {z: 3.14}}},
{x: {y: {z: 42}}},
],
};

```


## Error

```
Found 1 error:

Compilation Skipped: Existing memoization could not be preserved

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. The inferred dependency was `x.a.b?.c`, but the source dependencies were [x.a.b?.c.d?.e]. Inferred less specific property than source.

error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.ts:7:25
5 |
6 | function Component({x}) {
> 7 | const object = useMemo(() => {
| ^^^^^^^
> 8 | return identity({
| ^^^^^^^^^^^^^^^^^^^^^
> 9 | callback: () => {
| ^^^^^^^^^^^^^^^^^^^^^
> 10 | // This is a bug in our dependency inference: we stop capturing dependencies
| ^^^^^^^^^^^^^^^^^^^^^
> 11 | // after x.a.b?.c. But what this dependency is telling us is that if `x.a.b`
| ^^^^^^^^^^^^^^^^^^^^^
> 12 | // was non-nullish, then we can access `.c.d?.e`. Thus we should take the
| ^^^^^^^^^^^^^^^^^^^^^
> 13 | // full property chain, exactly as-is with optionals/non-optionals, as a
| ^^^^^^^^^^^^^^^^^^^^^
> 14 | // dependency
| ^^^^^^^^^^^^^^^^^^^^^
> 15 | return identity(x.a.b?.c.d?.e);
| ^^^^^^^^^^^^^^^^^^^^^
> 16 | },
| ^^^^^^^^^^^^^^^^^^^^^
> 17 | });
| ^^^^^^^^^^^^^^^^^^^^^
> 18 | }, [x.a.b?.c.d?.e]);
| ^^^^ Could not preserve existing manual memoization
19 | const result = useMemo(() => {
20 | return [object.callback()];
21 | }, [object]);
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees @enableOptionalDependencies @enableTreatFunctionDepsAsConditional:false

import {useMemo} from 'react';
import {identity, ValidateMemoization} from 'shared-runtime';

function Component({x}) {
const object = useMemo(() => {
return identity({
callback: () => {
// This is a bug in our dependency inference: we stop capturing dependencies
// after x.a.b?.c. But what this dependency is telling us is that if `x.a.b`
// was non-nullish, then we can access `.c.d?.e`. Thus we should take the
// full property chain, exactly as-is with optionals/non-optionals, as a
// dependency
return identity(x.a.b?.c.d?.e);
Comment on lines +10 to +15
Copy link
Member Author

@josephsavona josephsavona Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @mofeiZ this probably needs discussion. Even if we agree it's a bug after more consideration, this case is likely infrequent. So landing this PR would probably significantly reduce the number of remaining ValidatePreserveMemo bailouts due to deps not matching

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I wonder why this is still erroring. Could it be an off by one error -- x.a.b?.c is already marked as non-nullable

},
});
}, [x.a.b?.c.d?.e]);
const result = useMemo(() => {
return [object.callback()];
}, [object]);
return <Inner x={x} result={result} />;
}

function Inner({x, result}) {
'use no memo';
return <ValidateMemoization inputs={[x.y.z]} output={result} />;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{x: {y: {z: 42}}}],
sequentialRenders: [
{x: {y: {z: 42}}},
{x: {y: {z: 42}}},
{x: {y: {z: 3.14}}},
{x: {y: {z: 42}}},
{x: {y: {z: 3.14}}},
{x: {y: {z: 42}}},
],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@

## Input

```javascript
// @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees @enableOptionalDependencies @enableTreatFunctionDepsAsConditional:false

import {useMemo} from 'react';
import {identity, ValidateMemoization} from 'shared-runtime';

function Component({x}) {
const object = useMemo(() => {
return identity({
callback: () => {
return identity(x.y.z); // accesses more levels of properties than the manual memo
},
});
// x.y as a manual dep only tells us that x is non-nullable, not that x.y is non-nullable
// we can only take a dep on x.y, not x.y.z
}, [x.y]);
const result = useMemo(() => {
return [object.callback()];
}, [object]);
return <ValidateMemoization inputs={[x.y]} output={result} />;
}

const input1 = {x: {y: {z: 42}}};
const input1b = {x: {y: {z: 42}}};
const input2 = {x: {y: {z: 3.14}}};
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [input1],
sequentialRenders: [
input1,
input1,
input1b, // should reset even though .z didn't change
input1,
input2,
],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime"; // @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees @enableOptionalDependencies @enableTreatFunctionDepsAsConditional:false

import { useMemo } from "react";
import { identity, ValidateMemoization } from "shared-runtime";

function Component(t0) {
const $ = _c(11);
const { x } = t0;
let t1;
if ($[0] !== x.y) {
t1 = identity({ callback: () => identity(x.y.z) });
$[0] = x.y;
$[1] = t1;
} else {
t1 = $[1];
}
const object = t1;
let t2;
if ($[2] !== object) {
t2 = object.callback();
$[2] = object;
$[3] = t2;
} else {
t2 = $[3];
}
let t3;
if ($[4] !== t2) {
t3 = [t2];
$[4] = t2;
$[5] = t3;
} else {
t3 = $[5];
}
const result = t3;
let t4;
if ($[6] !== x.y) {
t4 = [x.y];
$[6] = x.y;
$[7] = t4;
} else {
t4 = $[7];
}
let t5;
if ($[8] !== result || $[9] !== t4) {
t5 = <ValidateMemoization inputs={t4} output={result} />;
$[8] = result;
$[9] = t4;
$[10] = t5;
} else {
t5 = $[10];
}
return t5;
}

const input1 = { x: { y: { z: 42 } } };
const input1b = { x: { y: { z: 42 } } };
const input2 = { x: { y: { z: 3.14 } } };
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [input1],
sequentialRenders: [
input1,
input1,
input1b, // should reset even though .z didn't change
input1,
input2,
],
};

```

### Eval output
(kind: ok) <div>{"inputs":[{"z":42}],"output":[42]}</div>
<div>{"inputs":[{"z":42}],"output":[42]}</div>
<div>{"inputs":[{"z":42}],"output":[42]}</div>
<div>{"inputs":[{"z":42}],"output":[42]}</div>
<div>{"inputs":[{"z":3.14}],"output":[3.14]}</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees @enableOptionalDependencies @enableTreatFunctionDepsAsConditional:false

import {useMemo} from 'react';
import {identity, ValidateMemoization} from 'shared-runtime';

function Component({x}) {
const object = useMemo(() => {
return identity({
callback: () => {
return identity(x.y.z); // accesses more levels of properties than the manual memo
},
});
// x.y as a manual dep only tells us that x is non-nullable, not that x.y is non-nullable
// we can only take a dep on x.y, not x.y.z
}, [x.y]);
const result = useMemo(() => {
return [object.callback()];
}, [object]);
return <ValidateMemoization inputs={[x.y]} output={result} />;
}

const input1 = {x: {y: {z: 42}}};
const input1b = {x: {y: {z: 42}}};
const input2 = {x: {y: {z: 3.14}}};
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [input1],
sequentialRenders: [
input1,
input1,
input1b, // should reset even though .z didn't change
input1,
input2,
],
};
Loading
Loading