From 5d64f742114203e4fbdd605ce398696d18901f35 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:44:42 -0700 Subject: [PATCH 1/3] [compiler] Fix for scopes with unreachable fallthroughs (#34335) Fixes #34108. If a scope ends with with a conditional where some/all branches exit via labeled break, we currently compile in a way that works but bypasses memoization. We end up with a shape like ```js let t0; label: { if (changed) { ... if (cond) { t0 = ...; break label; } // we don't save the output if the break happens! t0 = ...; $[0] = t0; } else { t0 = $[0]; } ``` The fix here is to update AlignReactiveScopesToBlockScopes to take account of breaks that don't go to the natural fallthrough. In this case, we take any active scopes and extend them to start at least as early as the label, and extend at least to the label fallthrough. Thus we produce the correct: ```js let t0; if (changed) { label: { ... if (cond) { t0 = ...; break label; } t0 = ...; } // now the break jumps here, and we cache the value $[0] = t0; } else { t0 = $[0]; } ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34335). * #34347 * #34346 * #34343 * __->__ #34335 --- .../AlignReactiveScopesToBlockScopesHIR.ts | 35 ++++++ ...copes-reactive-scope-overlaps-if.expect.md | 16 +-- .../mutation-within-jsx-and-break.expect.md | 19 +-- .../useMemo-multiple-if-else.expect.md | 37 +++--- ...seMemo-if-else-both-early-return.expect.md | 118 ++++++++++++++++++ ...repro-useMemo-if-else-both-early-return.js | 35 ++++++ .../useMemo-multiple-if-else.expect.md | 37 +++--- 7 files changed, 241 insertions(+), 56 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts index 2b4e890a40da8..e440340bd29f3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts @@ -175,6 +175,41 @@ export function alignReactiveScopesToBlockScopesHIR(fn: HIRFunction): void { if (node != null) { valueBlockNodes.set(fallthrough, node); } + } else if (terminal.kind === 'goto') { + /** + * If we encounter a goto that is not to the natural fallthrough of the current + * block (not the topmost fallthrough on the stack), then this is a goto to a + * label. Any scopes that extend beyond the goto must be extended to include + * the labeled range, so that the break statement doesn't accidentally jump + * out of the scope. We do this by extending the start and end of the scope's + * range to the label and its fallthrough respectively. + */ + const start = activeBlockFallthroughRanges.find( + range => range.fallthrough === terminal.block, + ); + if (start != null && start !== activeBlockFallthroughRanges.at(-1)) { + const fallthroughBlock = fn.body.blocks.get(start.fallthrough)!; + const firstId = + fallthroughBlock.instructions[0]?.id ?? fallthroughBlock.terminal.id; + for (const scope of activeScopes) { + /** + * activeScopes is only filtered at block start points, so some of the + * scopes may not actually be active anymore, ie we've past their end + * instruction. Only extend ranges for scopes that are actually active. + * + * TODO: consider pruning activeScopes per instruction + */ + if (scope.range.end <= terminal.id) { + continue; + } + scope.range.start = makeInstructionId( + Math.min(start.range.start, scope.range.start), + ); + scope.range.end = makeInstructionId( + Math.max(firstId, scope.range.end), + ); + } + } } /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/align-scopes-reactive-scope-overlaps-if.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/align-scopes-reactive-scope-overlaps-if.expect.md index 7136b3a173f61..03939d16d66e0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/align-scopes-reactive-scope-overlaps-if.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/align-scopes-reactive-scope-overlaps-if.expect.md @@ -46,14 +46,16 @@ function useFoo(t0) { t1 = $[0]; } let items = t1; - bb0: if ($[1] !== cond) { - if (cond) { - items = []; - } else { - break bb0; - } + if ($[1] !== cond) { + bb0: { + if (cond) { + items = []; + } else { + break bb0; + } - items.push(2); + items.push(2); + } $[1] = cond; $[2] = items; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mutation-within-jsx-and-break.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mutation-within-jsx-and-break.expect.md index 1d0f40e29f518..17a8524eeee8b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mutation-within-jsx-and-break.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mutation-within-jsx-and-break.expect.md @@ -49,12 +49,12 @@ import { } from "shared-runtime"; function useFoo(t0) { - const $ = _c(3); + const $ = _c(4); const { data } = t0; let obj; let myDiv = null; - bb0: if (data.cond) { - if ($[0] !== data.cond1) { + if ($[0] !== data.cond || $[1] !== data.cond1) { + bb0: if (data.cond) { obj = makeObject_Primitives(); if (data.cond1) { myDiv = ; @@ -62,13 +62,14 @@ function useFoo(t0) { } mutate(obj); - $[0] = data.cond1; - $[1] = obj; - $[2] = myDiv; - } else { - obj = $[1]; - myDiv = $[2]; } + $[0] = data.cond; + $[1] = data.cond1; + $[2] = obj; + $[3] = myDiv; + } else { + obj = $[2]; + myDiv = $[3]; } return myDiv; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/useMemo-multiple-if-else.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/useMemo-multiple-if-else.expect.md index a75b592b83232..4ce8ef58028cf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/useMemo-multiple-if-else.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/useMemo-multiple-if-else.expect.md @@ -34,17 +34,16 @@ import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR import { useMemo } from "react"; function Component(props) { - const $ = _c(6); + const $ = _c(5); let t0; - bb0: { - let y; - if ( - $[0] !== props.a || - $[1] !== props.b || - $[2] !== props.cond || - $[3] !== props.cond2 - ) { - y = []; + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.cond || + $[3] !== props.cond2 + ) { + bb0: { + const y = []; if (props.cond) { y.push(props.a); } @@ -54,17 +53,15 @@ function Component(props) { } y.push(props.b); - $[0] = props.a; - $[1] = props.b; - $[2] = props.cond; - $[3] = props.cond2; - $[4] = y; - $[5] = t0; - } else { - y = $[4]; - t0 = $[5]; + t0 = y; } - t0 = y; + $[0] = props.a; + $[1] = props.b; + $[2] = props.cond; + $[3] = props.cond2; + $[4] = t0; + } else { + t0 = $[4]; } const x = t0; return x; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.expect.md new file mode 100644 index 0000000000000..acb3b72e3a44b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.expect.md @@ -0,0 +1,118 @@ + +## Input + +```javascript +import {useMemo} from 'react'; +import { + makeObject_Primitives, + mutate, + Stringify, + ValidateMemoization, +} from 'shared-runtime'; + +function Component({cond}) { + const memoized = useMemo(() => { + const value = makeObject_Primitives(); + if (cond) { + return value; + } else { + mutate(value); + return value; + } + }, [cond]); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false}], + sequentialRenders: [ + {cond: false}, + {cond: false}, + {cond: true}, + {cond: true}, + {cond: false}, + {cond: true}, + {cond: false}, + {cond: true}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useMemo } from "react"; +import { + makeObject_Primitives, + mutate, + Stringify, + ValidateMemoization, +} from "shared-runtime"; + +function Component(t0) { + const $ = _c(7); + const { cond } = t0; + let t1; + if ($[0] !== cond) { + const value = makeObject_Primitives(); + if (cond) { + t1 = value; + } else { + mutate(value); + t1 = value; + } + $[0] = cond; + $[1] = t1; + } else { + t1 = $[1]; + } + const memoized = t1; + let t2; + if ($[2] !== cond) { + t2 = [cond]; + $[2] = cond; + $[3] = t2; + } else { + t2 = $[3]; + } + let t3; + if ($[4] !== memoized || $[5] !== t2) { + t3 = ; + $[4] = memoized; + $[5] = t2; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: false }], + sequentialRenders: [ + { cond: false }, + { cond: false }, + { cond: true }, + { cond: true }, + { cond: false }, + { cond: true }, + { cond: false }, + { cond: true }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":[false],"output":{"a":0,"b":"value1","c":true,"wat0":"joe"}}
+
{"inputs":[false],"output":{"a":0,"b":"value1","c":true,"wat0":"joe"}}
+
{"inputs":[true],"output":{"a":0,"b":"value1","c":true}}
+
{"inputs":[true],"output":{"a":0,"b":"value1","c":true}}
+
{"inputs":[false],"output":{"a":0,"b":"value1","c":true,"wat0":"joe"}}
+
{"inputs":[true],"output":{"a":0,"b":"value1","c":true}}
+
{"inputs":[false],"output":{"a":0,"b":"value1","c":true,"wat0":"joe"}}
+
{"inputs":[true],"output":{"a":0,"b":"value1","c":true}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.js new file mode 100644 index 0000000000000..d381661077131 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-useMemo-if-else-both-early-return.js @@ -0,0 +1,35 @@ +import {useMemo} from 'react'; +import { + makeObject_Primitives, + mutate, + Stringify, + ValidateMemoization, +} from 'shared-runtime'; + +function Component({cond}) { + const memoized = useMemo(() => { + const value = makeObject_Primitives(); + if (cond) { + return value; + } else { + mutate(value); + return value; + } + }, [cond]); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false}], + sequentialRenders: [ + {cond: false}, + {cond: false}, + {cond: true}, + {cond: true}, + {cond: false}, + {cond: true}, + {cond: false}, + {cond: true}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md index 23b05b548221f..7f4b8af9650f6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md @@ -33,17 +33,16 @@ import { c as _c } from "react/compiler-runtime"; import { useMemo } from "react"; function Component(props) { - const $ = _c(6); + const $ = _c(5); let t0; - bb0: { - let y; - if ( - $[0] !== props.a || - $[1] !== props.b || - $[2] !== props.cond || - $[3] !== props.cond2 - ) { - y = []; + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.cond || + $[3] !== props.cond2 + ) { + bb0: { + const y = []; if (props.cond) { y.push(props.a); } @@ -53,17 +52,15 @@ function Component(props) { } y.push(props.b); - $[0] = props.a; - $[1] = props.b; - $[2] = props.cond; - $[3] = props.cond2; - $[4] = y; - $[5] = t0; - } else { - y = $[4]; - t0 = $[5]; + t0 = y; } - t0 = y; + $[0] = props.a; + $[1] = props.b; + $[2] = props.cond; + $[3] = props.cond2; + $[4] = t0; + } else { + t0 = $[4]; } const x = t0; return x; From 735e9ac54e5b46e5a963e0e436330baf20fb04c2 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:45:17 -0700 Subject: [PATCH 2/3] [compiler] enablePreserveExistingMemo memoizes primitive-returning functions (#34343) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@enablePreserveExistingMemoizationGuarantees` mode currently does not guarantee memoization of primitive-returning functions. We're often able to infer that a function returns a primitive based on how its result is used, for example `foo() + 1` or `object[getIndex()]`, and by default we do not currently memoize computation that produces a primitive. The reasoning behind this is that the compiler is primarily focused on stopping cascading updates — it's fine to recompute a primitive since we can cheaply compare that primitive and avoid unnecessary downstream recomputation. But we've gotten a lot of feedback that people find this surprising, and that sometimes the computation can be expensive enough that it should be memoized. This PR changes `@enablePreserveExistingMemoizationGuarantees` mode to ensure that primitive-returning functions get memoized. Other modes will not memoize these functions. Separately from this we are considering enabling this mode by default. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34343). * #34347 * #34346 * __->__ #34343 * #34335 --- .../ReactiveScopes/PruneNonEscapingScopes.ts | 24 +++- ...nction-call-non-escaping-useMemo.expect.md | 77 +++++++++++++ ...tive-function-call-non-escaping-useMemo.js | 32 ++++++ ...itive-function-call-non-escaping.expect.md | 81 +++++++++++++ ...ze-primitive-function-call-non-escaping.js | 29 +++++ ...memoize-primitive-function-calls.expect.md | 107 ++++++++++++++++++ .../memoize-primitive-function-calls.js | 30 +++++ 7 files changed, 376 insertions(+), 4 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts index 3afb00b71a816..1dcaf0b798ea7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -411,7 +411,9 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor< this.state = state; this.options = { memoizeJsxElements: !this.env.config.enableForest, - forceMemoizePrimitives: this.env.config.enableForest, + forceMemoizePrimitives: + this.env.config.enableForest || + this.env.config.enablePreserveExistingMemoizationGuarantees, }; } @@ -534,9 +536,23 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor< case 'JSXText': case 'BinaryExpression': case 'UnaryExpression': { - const level = options.forceMemoizePrimitives - ? MemoizationLevel.Memoized - : MemoizationLevel.Never; + if (options.forceMemoizePrimitives) { + /** + * Because these instructions produce primitives we usually don't consider + * them as escape points: they are known to copy, not return references. + * However if we're forcing memoization of primitives then we mark these + * instructions as needing memoization and walk their rvalues to ensure + * any scopes transitively reachable from the rvalues are considered for + * memoization. Note: we may still prune primitive-producing scopes if + * they don't ultimately escape at all. + */ + const level = MemoizationLevel.Memoized; + return { + lvalues: lvalue !== null ? [{place: lvalue, level}] : [], + rvalues: [...eachReactiveValueOperand(value)], + }; + } + const level = MemoizationLevel.Never; return { // All of these instructions return a primitive value and never need to be memoized lvalues: lvalue !== null ? [{place: lvalue, level}] : [], diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.expect.md new file mode 100644 index 0000000000000..93b08128a0afe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.expect.md @@ -0,0 +1,77 @@ + +## Input + +```javascript +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import {useMemo} from 'react'; +import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const result = useMemo( + () => makeObject(props.value).value + 1, + [props.value] + ); + console.log(result); + return 'ok'; +} + +function makeObject(value) { + console.log(value); + return {value}; +} + +export const TODO_FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [ + {value: 42}, + {value: 42}, + {value: 3.14}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + ], +}; + +``` + +## Code + +```javascript +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import { useMemo } from "react"; +import { makeObject_Primitives, ValidateMemoization } from "shared-runtime"; + +function Component(props) { + const result = makeObject(props.value).value + 1; + + console.log(result); + return "ok"; +} + +function makeObject(value) { + console.log(value); + return { value }; +} + +export const TODO_FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 42 }], + sequentialRenders: [ + { value: 42 }, + { value: 42 }, + { value: 3.14 }, + { value: 3.14 }, + { value: 42 }, + { value: 3.14 }, + { value: 42 }, + { value: 3.14 }, + ], +}; + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.js new file mode 100644 index 0000000000000..2ee24917c53e6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.js @@ -0,0 +1,32 @@ +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import {useMemo} from 'react'; +import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const result = useMemo( + () => makeObject(props.value).value + 1, + [props.value] + ); + console.log(result); + return 'ok'; +} + +function makeObject(value) { + console.log(value); + return {value}; +} + +export const TODO_FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [ + {value: 42}, + {value: 42}, + {value: 3.14}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.expect.md new file mode 100644 index 0000000000000..e2f6c9e6c2ccf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.expect.md @@ -0,0 +1,81 @@ + +## Input + +```javascript +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import {useMemo} from 'react'; +import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const result = makeObject(props.value).value + 1; + console.log(result); + return 'ok'; +} + +function makeObject(value) { + console.log(value); + return {value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [ + {value: 42}, + {value: 42}, + {value: 3.14}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + ], +}; + +``` + +## Code + +```javascript +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import { useMemo } from "react"; +import { makeObject_Primitives, ValidateMemoization } from "shared-runtime"; + +function Component(props) { + const result = makeObject(props.value).value + 1; + console.log(result); + return "ok"; +} + +function makeObject(value) { + console.log(value); + return { value }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 42 }], + sequentialRenders: [ + { value: 42 }, + { value: 42 }, + { value: 3.14 }, + { value: 3.14 }, + { value: 42 }, + { value: 3.14 }, + { value: 42 }, + { value: 3.14 }, + ], +}; + +``` + +### Eval output +(kind: ok) "ok" +"ok" +"ok" +"ok" +"ok" +"ok" +"ok" +"ok" +logs: [42,43,42,43,3.14,4.140000000000001,3.14,4.140000000000001,42,43,3.14,4.140000000000001,42,43,3.14,4.140000000000001] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.js new file mode 100644 index 0000000000000..b4d8d344441c9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.js @@ -0,0 +1,29 @@ +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import {useMemo} from 'react'; +import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const result = makeObject(props.value).value + 1; + console.log(result); + return 'ok'; +} + +function makeObject(value) { + console.log(value); + return {value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [ + {value: 42}, + {value: 42}, + {value: 3.14}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.expect.md new file mode 100644 index 0000000000000..70e70a26b4656 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.expect.md @@ -0,0 +1,107 @@ + +## Input + +```javascript +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import {useMemo} from 'react'; +import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const result = useMemo(() => { + return makeObject(props.value).value + 1; + }, [props.value]); + return ; +} + +function makeObject(value) { + console.log(value); + return {value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [ + {value: 42}, + {value: 42}, + {value: 3.14}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import { useMemo } from "react"; +import { makeObject_Primitives, ValidateMemoization } from "shared-runtime"; + +function Component(props) { + const $ = _c(7); + let t0; + if ($[0] !== props.value) { + t0 = makeObject(props.value); + $[0] = props.value; + $[1] = t0; + } else { + t0 = $[1]; + } + const result = t0.value + 1; + let t1; + if ($[2] !== props.value) { + t1 = [props.value]; + $[2] = props.value; + $[3] = t1; + } else { + t1 = $[3]; + } + let t2; + if ($[4] !== result || $[5] !== t1) { + t2 = ; + $[4] = result; + $[5] = t1; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} + +function makeObject(value) { + console.log(value); + return { value }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 42 }], + sequentialRenders: [ + { value: 42 }, + { value: 42 }, + { value: 3.14 }, + { value: 3.14 }, + { value: 42 }, + { value: 3.14 }, + { value: 42 }, + { value: 3.14 }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":[42],"output":43}
+
{"inputs":[42],"output":43}
+
{"inputs":[3.14],"output":4.140000000000001}
+
{"inputs":[3.14],"output":4.140000000000001}
+
{"inputs":[42],"output":43}
+
{"inputs":[3.14],"output":4.140000000000001}
+
{"inputs":[42],"output":43}
+
{"inputs":[3.14],"output":4.140000000000001}
+logs: [42,3.14,42,3.14,42,3.14] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.js new file mode 100644 index 0000000000000..5c7cefb609c46 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.js @@ -0,0 +1,30 @@ +// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees +import {useMemo} from 'react'; +import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime'; + +function Component(props) { + const result = useMemo(() => { + return makeObject(props.value).value + 1; + }, [props.value]); + return ; +} + +function makeObject(value) { + console.log(value); + return {value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [ + {value: 42}, + {value: 42}, + {value: 3.14}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + {value: 42}, + {value: 3.14}, + ], +}; From 2710795a1ed339764d2fa76b6d7bc94dded6ee60 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Wed, 3 Sep 2025 21:30:52 -0700 Subject: [PATCH 3/3] [compiler] Cleanup for @enablePreserveExistingMemoizationGuarantees (#34346) I tried turning on `@enablePreserveExistingMemoizationGuarantees` by default and cleaned up a couple small things: * We emit freeze calls for StartMemoize deps but these had ValueReason.Other so the message wasn't great. We now treat these like other hook arguments. * PruneNonEscapingScopes was being too aggressive in this mode and memoizing even loads of globals. Switching to MemoizationLevel.Conditional ensures we build a graph that connects through to primitive-returning function calls, but doesn't unnecessarily force memoization otherwise. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34346). * #34347 * __->__ #34346 --- .../Inference/InferMutationAliasingEffects.ts | 2 +- .../ReactiveScopes/PruneNonEscapingScopes.ts | 6 +- .../repro-cx-namespace-nesting.expect.md | 3 +- .../meta-isms/repro-cx-namespace-nesting.js | 1 + ...da-with-fbt-preserve-memoization.expect.md | 86 +++++++++++++++++++ .../lambda-with-fbt-preserve-memoization.js | 31 +++++++ ...signed-loop-force-scopes-enabled.expect.md | 30 +++---- ...ve-reassigned-loop-force-scopes-enabled.js | 2 +- 8 files changed, 138 insertions(+), 23 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-existing-memoization-guarantees/lambda-with-fbt-preserve-memoization.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-existing-memoization-guarantees/lambda-with-fbt-preserve-memoization.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 8ef78aa196428..a0e9593268812 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -2089,7 +2089,7 @@ function computeSignatureForInstruction( effects.push({ kind: 'Freeze', value: operand, - reason: ValueReason.Other, + reason: ValueReason.HookCaptured, }); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts index 1dcaf0b798ea7..5735f7e80115b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -546,7 +546,7 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor< * memoization. Note: we may still prune primitive-producing scopes if * they don't ultimately escape at all. */ - const level = MemoizationLevel.Memoized; + const level = MemoizationLevel.Conditional; return { lvalues: lvalue !== null ? [{place: lvalue, level}] : [], rvalues: [...eachReactiveValueOperand(value)], @@ -701,9 +701,7 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor< } case 'ComputedLoad': case 'PropertyLoad': { - const level = options.forceMemoizePrimitives - ? MemoizationLevel.Memoized - : MemoizationLevel.Conditional; + const level = MemoizationLevel.Conditional; return { // Indirection for the inner value, memoized if the value is lvalues: lvalue !== null ? [{place: lvalue, level}] : [], diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md index 7f1fb96617e33..90de08f333520 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @compilationMode:"infer" import {makeArray} from 'shared-runtime'; function Component() { @@ -30,7 +31,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer" import { makeArray } from "shared-runtime"; function Component() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js index 352e2e5c19ba7..41aebae7e335d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js @@ -1,3 +1,4 @@ +// @compilationMode:"infer" import {makeArray} from 'shared-runtime'; function Component() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-existing-memoization-guarantees/lambda-with-fbt-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-existing-memoization-guarantees/lambda-with-fbt-preserve-memoization.expect.md new file mode 100644 index 0000000000000..bafbb5c5ef37a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-existing-memoization-guarantees/lambda-with-fbt-preserve-memoization.expect.md @@ -0,0 +1,86 @@ + +## Input + +```javascript +// @enablePreserveExistingMemoizationGuarantees +import {fbt} from 'fbt'; + +function Component() { + const buttonLabel = () => { + if (!someCondition) { + return {'Purchase as a gift'}; + } else if ( + !iconOnly && + showPrice && + item?.current_gift_offer?.price?.formatted != null + ) { + return ( + + {'Gift | '} + + {item?.current_gift_offer?.price?.formatted} + + + ); + } else if (!iconOnly && !showPrice) { + return {'Gift'}; + } + }; + + return ( + +