diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index f249466431..7bb65fbc04 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -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); + } + } + } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md new file mode 100644 index 0000000000..e9772e6799 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.expect.md @@ -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 ; +} + +function Inner({x, result}) { + 'use no memo'; + return ; +} + +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]); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.js new file mode 100644 index 0000000000..12f8ebf3ce --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-preserve-memo-deps-mixed-optional-nonoptional-property-chain.js @@ -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); + }, + }); + }, [x.a.b?.c.d?.e]); + const result = useMemo(() => { + return [object.callback()]; + }, [object]); + return ; +} + +function Inner({x, result}) { + 'use no memo'; + return ; +} + +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}}}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain-less-precise-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain-less-precise-deps.expect.md new file mode 100644 index 0000000000..84c611dec3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain-less-precise-deps.expect.md @@ -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 ; +} + +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 = ; + $[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)
{"inputs":[{"z":42}],"output":[42]}
+
{"inputs":[{"z":42}],"output":[42]}
+
{"inputs":[{"z":42}],"output":[42]}
+
{"inputs":[{"z":42}],"output":[42]}
+
{"inputs":[{"z":3.14}],"output":[3.14]}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain-less-precise-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain-less-precise-deps.js new file mode 100644 index 0000000000..373fdc53fa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain-less-precise-deps.js @@ -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 ; +} + +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, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain.expect.md new file mode 100644 index 0000000000..82c11f7783 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain.expect.md @@ -0,0 +1,117 @@ + +## 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); + }, + }); + }, [x.y.z]); + const result = useMemo(() => { + return [object.callback()]; + }, [object]); + return ; +} + +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}}}, + ], +}; + +``` + +## 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.z) { + t1 = identity({ callback: () => identity(x.y.z) }); + $[0] = x.y.z; + $[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.z) { + t4 = [x.y.z]; + $[6] = x.y.z; + $[7] = t4; + } else { + t4 = $[7]; + } + let t5; + if ($[8] !== result || $[9] !== t4) { + t5 = ; + $[8] = result; + $[9] = t4; + $[10] = t5; + } else { + t5 = $[10]; + } + return t5; +} + +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 } } }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":[42],"output":[42]}
+
{"inputs":[42],"output":[42]}
+
{"inputs":[3.14],"output":[3.14]}
+
{"inputs":[42],"output":[42]}
+
{"inputs":[3.14],"output":[3.14]}
+
{"inputs":[42],"output":[42]}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain.js new file mode 100644 index 0000000000..6b55e68bb0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-conditional-property-chain.js @@ -0,0 +1,31 @@ +// @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); + }, + }); + }, [x.y.z]); + const result = useMemo(() => { + return [object.callback()]; + }, [object]); + return ; +} + +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}}}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-optional-property-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-optional-property-chain.expect.md new file mode 100644 index 0000000000..ab4940bcc3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-optional-property-chain.expect.md @@ -0,0 +1,125 @@ + +## Input + +```javascript +// @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees @enableOptionalDependencies @enableTreatFunctionDepsAsConditional:false + +import {useMemo} from 'react'; +import {identity, ValidateMemoization} from 'shared-runtime'; + +function Component({x, y, z}) { + const object = useMemo(() => { + return identity({ + callback: () => { + return identity(x?.y?.z, y.a?.b, z.a.b?.c); + }, + }); + }, [x?.y?.z, y.a?.b, z.a.b?.c]); + const result = useMemo(() => { + return [object.callback()]; + }, [object]); + return ; +} + +function Inner({x, result}) { + 'use no memo'; + return ; +} + +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}}}, + ], +}; + +``` + +## 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, y, z } = t0; + + x?.y?.z; + y.a?.b; + z.a.b?.c; + let t1; + if ($[0] !== x?.y?.z || $[1] !== y.a?.b || $[2] !== z.a.b?.c) { + t1 = identity({ callback: () => identity(x?.y?.z, y.a?.b, z.a.b?.c) }); + $[0] = x?.y?.z; + $[1] = y.a?.b; + $[2] = z.a.b?.c; + $[3] = t1; + } else { + t1 = $[3]; + } + const object = t1; + let t2; + if ($[4] !== object) { + t2 = object.callback(); + $[4] = object; + $[5] = t2; + } else { + t2 = $[5]; + } + let t3; + if ($[6] !== t2) { + t3 = [t2]; + $[6] = t2; + $[7] = t3; + } else { + t3 = $[7]; + } + const result = t3; + let t4; + if ($[8] !== result || $[9] !== x) { + t4 = ; + $[8] = result; + $[9] = x; + $[10] = t4; + } else { + t4 = $[10]; + } + return t4; +} + +function Inner({ x, result }) { + "use no memo"; + return ; +} + +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 } } }, + ], +}; + +``` + +### Eval output +(kind: ok) [[ (exception in render) TypeError: Cannot read properties of undefined (reading 'a') ]] +[[ (exception in render) TypeError: Cannot read properties of undefined (reading 'a') ]] +[[ (exception in render) TypeError: Cannot read properties of undefined (reading 'a') ]] +[[ (exception in render) TypeError: Cannot read properties of undefined (reading 'a') ]] +[[ (exception in render) TypeError: Cannot read properties of undefined (reading 'a') ]] +[[ (exception in render) TypeError: Cannot read properties of undefined (reading 'a') ]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-optional-property-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-optional-property-chain.js new file mode 100644 index 0000000000..820cef20cb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-deps-optional-property-chain.js @@ -0,0 +1,36 @@ +// @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees @enableOptionalDependencies @enableTreatFunctionDepsAsConditional:false + +import {useMemo} from 'react'; +import {identity, ValidateMemoization} from 'shared-runtime'; + +function Component({x, y, z}) { + const object = useMemo(() => { + return identity({ + callback: () => { + return identity(x?.y?.z, y.a?.b, z.a.b?.c); + }, + }); + }, [x?.y?.z, y.a?.b, z.a.b?.c]); + const result = useMemo(() => { + return [object.callback()]; + }, [object]); + return ; +} + +function Inner({x, result}) { + 'use no memo'; + return ; +} + +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}}}, + ], +};