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] [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 | 32 ++++--- ...ve-reassigned-loop-force-scopes-enabled.js | 2 +- 8 files changed, 139 insertions(+), 24 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 8ef78aa196..a0e9593268 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 1dcaf0b798..5735f7e801 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 7f1fb96617..90de08f333 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 352e2e5c19..41aebae7e3 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 0000000000..bafbb5c5ef --- /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 ( + +