From 6b70072c4f21d6762d914adb42007db68f1e00a9 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 9 Sep 2025 17:56:26 +0200 Subject: [PATCH 1/5] [DevTools] Finalize heuristic for naming unnamed `` (#34428) --- .../profilingCommitTreeBuilder-test.js | 6 ++-- .../src/__tests__/store-test.js | 30 +++++++++---------- .../src/__tests__/treeContext-test.js | 8 ++--- .../src/devtools/store.js | 11 ++++--- 4 files changed, 27 insertions(+), 28 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js b/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js index 6716766b2794c..73df17071bac2 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js @@ -229,7 +229,7 @@ describe('commit tree', () => { ▾ [suspense-root] rects={null} - + `); utils.act(() => modernRender()); expect(store).toMatchInlineSnapshot(` @@ -238,7 +238,7 @@ describe('commit tree', () => { ▾ [suspense-root] rects={null} - + `); utils.act(() => modernRender()); expect(store).toMatchInlineSnapshot(` @@ -304,7 +304,7 @@ describe('commit tree', () => { ▾ [suspense-root] rects={null} - + `); utils.act(() => modernRender()); expect(store).toMatchInlineSnapshot(` diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index e6eddff8402f0..fad4622acc15b 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -493,7 +493,7 @@ describe('Store', () => { ▾ [suspense-root] rects={[{x:1,y:2,width:5,height:1}, {x:1,y:2,width:10,height:1}]} - + `); await act(() => { @@ -506,7 +506,7 @@ describe('Store', () => { ▾ [suspense-root] rects={[{x:1,y:2,width:5,height:1}, {x:1,y:2,width:5,height:1}]} - + `); }); @@ -1054,7 +1054,7 @@ describe('Store', () => { ▾ [suspense-root] rects={[{x:1,y:2,width:5,height:1}, {x:1,y:2,width:10,height:1}]} - + `); await act(() => { @@ -1069,7 +1069,7 @@ describe('Store', () => { [suspense-root] rects={[{x:1,y:2,width:5,height:1}, {x:1,y:2,width:5,height:1}, {x:1,y:2,width:5,height:1}]} - + `); }); @@ -1407,7 +1407,7 @@ describe('Store', () => { [root] ▸ [suspense-root] rects={[{x:1,y:2,width:5,height:1}, {x:1,y:2,width:10,height:1}]} - + `); // This test isn't meaningful unless we expand the suspended tree @@ -1424,7 +1424,7 @@ describe('Store', () => { ▾ [suspense-root] rects={[{x:1,y:2,width:5,height:1}, {x:1,y:2,width:10,height:1}]} - + `); await act(() => { @@ -1437,7 +1437,7 @@ describe('Store', () => { ▾ [suspense-root] rects={[{x:1,y:2,width:5,height:1}, {x:1,y:2,width:5,height:1}]} - + `); }); @@ -1663,7 +1663,7 @@ describe('Store', () => { [root] ▸ [suspense-root] rects={null} - + `); await act(() => @@ -1678,7 +1678,7 @@ describe('Store', () => { ▾ ▸ [suspense-root] rects={null} - + `); const rendererID = getRendererID(); @@ -1697,7 +1697,7 @@ describe('Store', () => { ▾ [suspense-root] rects={null} - + `); await act(() => @@ -1713,7 +1713,7 @@ describe('Store', () => { ▾ ▸ [suspense-root] rects={null} - + `); }); }); @@ -2018,7 +2018,7 @@ describe('Store', () => { ▾ [suspense-root] rects={null} - + `); await Promise.resolve(); @@ -2032,7 +2032,7 @@ describe('Store', () => { ▾ [suspense-root] rects={null} - + `); // Render again to unmount it @@ -2523,7 +2523,7 @@ describe('Store', () => { ▾ [suspense-root] rects={null} - + `); await actAsync(() => render()); @@ -2534,7 +2534,7 @@ describe('Store', () => { ▾ [suspense-root] rects={null} - + `); }); }); diff --git a/packages/react-devtools-shared/src/__tests__/treeContext-test.js b/packages/react-devtools-shared/src/__tests__/treeContext-test.js index 6f382f976cb0a..9b6b7e7ab073d 100644 --- a/packages/react-devtools-shared/src/__tests__/treeContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/treeContext-test.js @@ -1369,8 +1369,8 @@ describe('TreeListContext', () => { ▾ [suspense-root] rects={null} - - + + `); const outerSuspenseID = ((store.getElementIDAtIndex(1): any): number); @@ -1411,8 +1411,8 @@ describe('TreeListContext', () => { ▾ [suspense-root] rects={null} - - + + `); }); }); diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 02e60a080af3e..310321b5ffcc5 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -1950,14 +1950,13 @@ export default class Store extends EventEmitter<{ throw error; } - _guessSuspenseName(element: Element): string | null { - // TODO: Use key + _guessSuspenseName(element: Element): string { const owner = this._idToElement.get(element.ownerID); - if (owner !== undefined) { - // TODO: This is clowny - return `${owner.displayName || 'Unknown'}>?`; + let name = 'Unknown'; + if (owner !== undefined && owner.displayName !== null) { + name = owner.displayName; } - return null; + return name; } } From a9410fb487776339ec68e57a57a570be952ccad0 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Tue, 9 Sep 2025 10:22:19 -0700 Subject: [PATCH 2/5] [compiler] Option to infer names for anonymous functions (#34410) Adds a `@enableNameAnonymousFunctions` feature to infer helpful names for anonymous functions within components and hooks. The logic is inspired by a custom Next.js transform, flagged to us by @eps1lon, that does something similar. Implementing this transform within React Compiler means that all React (Compiler) users can benefit from more helpful names when debugging. The idea builds on the fact that JS engines try to infer helpful names for anonymous functions (in stack traces) when those functions are accessed through an object property lookup: ```js ({'a[xyz]': () => { throw new Error('hello!') } }['a[xyz]'])() // Stack trace: Uncaught Error: hello! at a[xyz] (:1:26) // <-- note the name here at :1:60 ``` The new NameAnonymousFunctions transform is gated by the above flag, which is off by default. It attemps to infer names for functions as follows: First, determine a "local" name: * Assigning a function to a named variable uses the variable name. `const f = () => {}` gets the name "f". * Passing the function as an argument to a function gets the name of the function, ie `foo(() => ...)` get the name "foo()", `foo.bar(() => ...)` gets the name "foo.bar()". Note the parenthesis to help understand that it was part of a call. * Passing the function to a known hook uses the name of the hook, `useEffect(() => ...)` uses "useEffect()". * Passing the function as a JSX prop uses the element and attr name, eg `
...}` uses "
.onClick". Second, the local name is combined with the name of the outer component/hook, so the final names will be strings like `Component[f]` or `useMyHook[useEffect()]`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34410). * #34434 * __->__ #34410 --- .../src/Entrypoint/Pipeline.ts | 10 + .../src/HIR/BuildHIR.ts | 2 + .../src/HIR/Environment.ts | 2 + .../src/HIR/HIR.ts | 10 + .../ReactiveScopes/CodegenReactiveFunction.ts | 24 +- .../src/Transform/NameAnonymousFunctions.ts | 173 +++++++++++ .../name-anonymous-functions.expect.md | 272 ++++++++++++++++++ .../compiler/name-anonymous-functions.js | 53 ++++ 8 files changed, 544 insertions(+), 2 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index e5005d02c4ef7..7f8640122b942 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -103,6 +103,7 @@ import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoF import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects'; import {inferMutationAliasingRanges} from '../Inference/InferMutationAliasingRanges'; import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDerivedComputationsInEffects'; +import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -414,6 +415,15 @@ function runWithEnvironment( }); } + if (env.config.enableNameAnonymousFunctions) { + nameAnonymousFunctions(hir); + log({ + kind: 'hir', + name: 'NameAnonymougFunctions', + value: hir, + }); + } + const reactiveFunction = buildReactiveFunction(hir); log({ kind: 'reactive', diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index 02607f27b4e60..4d0461fe105b0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -3566,6 +3566,8 @@ function lowerFunctionToValue( let name: string | null = null; if (expr.isFunctionExpression()) { name = expr.get('id')?.node?.name ?? null; + } else if (expr.isFunctionDeclaration()) { + name = expr.get('id')?.node?.name ?? null; } const loweredFunc = lowerFunction(builder, expr); if (!loweredFunc) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index e38a49cb56a06..8e6816a3d5128 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -261,6 +261,8 @@ export const EnvironmentConfigSchema = z.object({ enableFire: z.boolean().default(false), + enableNameAnonymousFunctions: z.boolean().default(false), + /** * Enables inference and auto-insertion of effect dependencies. Takes in an array of * configurable module and import pairs to allow for user-land experimentation. For example, diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 582ef93cf34f2..fa502f821d0b7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -15,6 +15,7 @@ import {Type, makeType} from './Types'; import {z} from 'zod'; import type {AliasingEffect} from '../Inference/AliasingEffects'; import {isReservedWord} from '../Utils/Keyword'; +import {Err, Ok, Result} from '../Utils/Result'; /* * ******************************************************************************************* @@ -1298,6 +1299,15 @@ export function forkTemporaryIdentifier( }; } +export function validateIdentifierName( + name: string, +): Result { + if (isReservedWord(name) || !t.isValidIdentifier(name)) { + return Err(null); + } + return Ok(makeIdentifierName(name).value); +} + /** * Creates a valid identifier name. This should *not* be used for synthesizing * identifier names: only call this method for identifier names that appear in the diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 9d6f46c2374a7..28d8afd84b4fe 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -43,6 +43,7 @@ import { ValidIdentifierName, getHookKind, makeIdentifierName, + validateIdentifierName, } from '../HIR/HIR'; import {printIdentifier, printInstruction, printPlace} from '../HIR/PrintHIR'; import {eachPatternOperand} from '../HIR/visitors'; @@ -2326,6 +2327,11 @@ function codegenInstructionValue( ), reactiveFunction, ).unwrap(); + + const validatedName = + instrValue.name != null + ? validateIdentifierName(instrValue.name) + : Err(null); if (instrValue.type === 'ArrowFunctionExpression') { let body: t.BlockStatement | t.Expression = fn.body; if (body.body.length === 1 && loweredFunc.directives.length == 0) { @@ -2337,14 +2343,28 @@ function codegenInstructionValue( value = t.arrowFunctionExpression(fn.params, body, fn.async); } else { value = t.functionExpression( - fn.id ?? - (instrValue.name != null ? t.identifier(instrValue.name) : null), + validatedName + .map(name => t.identifier(name)) + .unwrapOr(null), fn.params, fn.body, fn.generator, fn.async, ); } + if ( + cx.env.config.enableNameAnonymousFunctions && + validatedName.isErr() && + instrValue.name != null + ) { + const name = instrValue.name; + value = t.memberExpression( + t.objectExpression([t.objectProperty(t.stringLiteral(name), value)]), + t.stringLiteral(name), + true, + false, + ); + } break; } case 'TaggedTemplateExpression': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts new file mode 100644 index 0000000000000..5fed3af0f733f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts @@ -0,0 +1,173 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { + FunctionExpression, + getHookKind, + HIRFunction, + IdentifierId, +} from '../HIR'; + +export function nameAnonymousFunctions(fn: HIRFunction): void { + if (fn.id == null) { + return; + } + const parentName = fn.id; + const functions = nameAnonymousFunctionsImpl(fn); + function visit(node: Node, prefix: string): void { + if (node.generatedName != null) { + /** + * Note that we don't generate a name for functions that already had one, + * so we'll only add the prefix to anonymous functions regardless of + * nesting depth. + */ + const name = `${prefix}${node.generatedName}]`; + node.fn.name = name; + } + /** + * Whether or not we generated a name for the function at this node, + * traverse into its nested functions to assign them names + */ + const nextPrefix = `${prefix}${node.generatedName ?? node.fn.name ?? ''} > `; + for (const inner of node.inner) { + visit(inner, nextPrefix); + } + } + for (const node of functions) { + visit(node, `${parentName}[`); + } +} + +type Node = { + fn: FunctionExpression; + generatedName: string | null; + inner: Array; +}; + +function nameAnonymousFunctionsImpl(fn: HIRFunction): Array { + // Functions that we track to generate names for + const functions: Map = new Map(); + // Tracks temporaries that read from variables/globals/properties + const names: Map = new Map(); + // Tracks all function nodes to bubble up for later renaming + const nodes: Array = []; + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const {lvalue, value} = instr; + switch (value.kind) { + case 'LoadGlobal': { + names.set(lvalue.identifier.id, value.binding.name); + break; + } + case 'LoadContext': + case 'LoadLocal': { + const name = value.place.identifier.name; + if (name != null && name.kind === 'named') { + names.set(lvalue.identifier.id, name.value); + } + break; + } + case 'PropertyLoad': { + const objectName = names.get(value.object.identifier.id); + if (objectName != null) { + names.set( + lvalue.identifier.id, + `${objectName}.${String(value.property)}`, + ); + } + break; + } + case 'FunctionExpression': { + const inner = nameAnonymousFunctionsImpl(value.loweredFunc.func); + const node: Node = { + fn: value, + generatedName: null, + inner, + }; + /** + * Bubble-up all functions, even if they're named, so that we can + * later generate names for any inner anonymous functions + */ + nodes.push(node); + if (value.name == null) { + // but only generate names for anonymous functions + functions.set(lvalue.identifier.id, node); + } + break; + } + case 'StoreContext': + case 'StoreLocal': { + const node = functions.get(value.value.identifier.id); + const variableName = value.lvalue.place.identifier.name; + if ( + node != null && + variableName != null && + variableName.kind === 'named' + ) { + node.generatedName = variableName.value; + functions.delete(value.value.identifier.id); + } + break; + } + case 'CallExpression': + case 'MethodCall': { + const callee = + value.kind === 'MethodCall' ? value.property : value.callee; + const hookKind = getHookKind(fn.env, callee.identifier); + let calleeName: string | null = null; + if (hookKind != null && hookKind !== 'Custom') { + calleeName = hookKind; + } else { + calleeName = names.get(callee.identifier.id) ?? '(anonymous)'; + } + let fnArgCount = 0; + for (const arg of value.args) { + if (arg.kind === 'Identifier' && functions.has(arg.identifier.id)) { + fnArgCount++; + } + } + for (let i = 0; i < value.args.length; i++) { + const arg = value.args[i]!; + if (arg.kind === 'Spread') { + continue; + } + const node = functions.get(arg.identifier.id); + if (node != null) { + const generatedName = + fnArgCount > 1 ? `${calleeName}(arg${i})` : `${calleeName}()`; + node.generatedName = generatedName; + functions.delete(arg.identifier.id); + } + } + break; + } + case 'JsxExpression': { + for (const attr of value.props) { + if (attr.kind === 'JsxSpreadAttribute') { + continue; + } + const node = functions.get(attr.place.identifier.id); + if (node != null) { + const elementName = + value.tag.kind === 'BuiltinTag' + ? value.tag.name + : (names.get(value.tag.identifier.id) ?? null); + const propName = + elementName == null + ? attr.name + : `<${elementName}>.${attr.name}`; + node.generatedName = `${propName}`; + functions.delete(attr.place.identifier.id); + } + } + break; + } + } + } + } + return nodes; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions.expect.md new file mode 100644 index 0000000000000..88e270647d3e6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions.expect.md @@ -0,0 +1,272 @@ + +## Input + +```javascript +// @enableNameAnonymousFunctions + +import {useEffect} from 'react'; +import {identity, Stringify, useIdentity} from 'shared-runtime'; +import * as SharedRuntime from 'shared-runtime'; + +function Component(props) { + function named() { + const inner = () => props.named; + return inner(); + } + const namedVariable = function () { + return props.namedVariable; + }; + const methodCall = SharedRuntime.identity(() => props.methodCall); + const call = identity(() => props.call); + const builtinElementAttr =
props.builtinElementAttr} />; + const namedElementAttr = props.namedElementAttr} />; + const hookArgument = useIdentity(() => props.hookArgument); + useEffect(() => { + console.log(props.useEffect); + JSON.stringify(null, null, () => props.useEffect); + const g = () => props.useEffect; + console.log(g()); + }, [props.useEffect]); + return ( + <> + {named()} + {namedVariable()} + {methodCall()} + {call()} + {builtinElementAttr} + {namedElementAttr} + {hookArgument()} + + ); +} + +export const TODO_FIXTURE_ENTRYPOINT = { + fn: Component, + params: [ + { + named: '', + namedVariable: '', + methodCall: '', + call: '', + builtinElementAttr: '', + namedElementAttr: '', + hookArgument: '', + useEffect: '', + }, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNameAnonymousFunctions + +import { useEffect } from "react"; +import { identity, Stringify, useIdentity } from "shared-runtime"; +import * as SharedRuntime from "shared-runtime"; + +function Component(props) { + const $ = _c(31); + let t0; + if ($[0] !== props.named) { + t0 = function named() { + const inner = { "Component[named > inner]": () => props.named }[ + "Component[named > inner]" + ]; + return inner(); + }; + $[0] = props.named; + $[1] = t0; + } else { + t0 = $[1]; + } + const named = t0; + let t1; + if ($[2] !== props.namedVariable) { + t1 = { + "Component[namedVariable]": function () { + return props.namedVariable; + }, + }["Component[namedVariable]"]; + $[2] = props.namedVariable; + $[3] = t1; + } else { + t1 = $[3]; + } + const namedVariable = t1; + let t2; + if ($[4] !== props.methodCall) { + t2 = { "Component[SharedRuntime.identity()]": () => props.methodCall }[ + "Component[SharedRuntime.identity()]" + ]; + $[4] = props.methodCall; + $[5] = t2; + } else { + t2 = $[5]; + } + const methodCall = SharedRuntime.identity(t2); + let t3; + if ($[6] !== props.call) { + t3 = { "Component[identity()]": () => props.call }["Component[identity()]"]; + $[6] = props.call; + $[7] = t3; + } else { + t3 = $[7]; + } + const call = identity(t3); + let t4; + if ($[8] !== props.builtinElementAttr) { + t4 = ( +
.onClick]": () => props.builtinElementAttr }[ + "Component[
.onClick]" + ] + } + /> + ); + $[8] = props.builtinElementAttr; + $[9] = t4; + } else { + t4 = $[9]; + } + const builtinElementAttr = t4; + let t5; + if ($[10] !== props.namedElementAttr) { + t5 = ( + .onClick]": () => props.namedElementAttr }[ + "Component[.onClick]" + ] + } + /> + ); + $[10] = props.namedElementAttr; + $[11] = t5; + } else { + t5 = $[11]; + } + const namedElementAttr = t5; + let t6; + if ($[12] !== props.hookArgument) { + t6 = { "Component[useIdentity()]": () => props.hookArgument }[ + "Component[useIdentity()]" + ]; + $[12] = props.hookArgument; + $[13] = t6; + } else { + t6 = $[13]; + } + const hookArgument = useIdentity(t6); + let t7; + let t8; + if ($[14] !== props.useEffect) { + t7 = { + "Component[useEffect()]": () => { + console.log(props.useEffect); + JSON.stringify( + null, + null, + { + "Component[useEffect() > JSON.stringify()]": () => props.useEffect, + }["Component[useEffect() > JSON.stringify()]"], + ); + const g = { "Component[useEffect() > g]": () => props.useEffect }[ + "Component[useEffect() > g]" + ]; + console.log(g()); + }, + }["Component[useEffect()]"]; + t8 = [props.useEffect]; + $[14] = props.useEffect; + $[15] = t7; + $[16] = t8; + } else { + t7 = $[15]; + t8 = $[16]; + } + useEffect(t7, t8); + let t9; + if ($[17] !== named) { + t9 = named(); + $[17] = named; + $[18] = t9; + } else { + t9 = $[18]; + } + let t10; + if ($[19] !== namedVariable) { + t10 = namedVariable(); + $[19] = namedVariable; + $[20] = t10; + } else { + t10 = $[20]; + } + const t11 = methodCall(); + const t12 = call(); + let t13; + if ($[21] !== hookArgument) { + t13 = hookArgument(); + $[21] = hookArgument; + $[22] = t13; + } else { + t13 = $[22]; + } + let t14; + if ( + $[23] !== builtinElementAttr || + $[24] !== namedElementAttr || + $[25] !== t10 || + $[26] !== t11 || + $[27] !== t12 || + $[28] !== t13 || + $[29] !== t9 + ) { + t14 = ( + <> + {t9} + {t10} + {t11} + {t12} + {builtinElementAttr} + {namedElementAttr} + {t13} + + ); + $[23] = builtinElementAttr; + $[24] = namedElementAttr; + $[25] = t10; + $[26] = t11; + $[27] = t12; + $[28] = t13; + $[29] = t9; + $[30] = t14; + } else { + t14 = $[30]; + } + return t14; +} + +export const TODO_FIXTURE_ENTRYPOINT = { + fn: Component, + params: [ + { + named: "", + namedVariable: "", + methodCall: "", + call: "", + builtinElementAttr: "", + namedElementAttr: "", + hookArgument: "", + useEffect: "", + }, + ], +}; + +``` + +### 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/name-anonymous-functions.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions.js new file mode 100644 index 0000000000000..ff4f1f6017d98 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions.js @@ -0,0 +1,53 @@ +// @enableNameAnonymousFunctions + +import {useEffect} from 'react'; +import {identity, Stringify, useIdentity} from 'shared-runtime'; +import * as SharedRuntime from 'shared-runtime'; + +function Component(props) { + function named() { + const inner = () => props.named; + return inner(); + } + const namedVariable = function () { + return props.namedVariable; + }; + const methodCall = SharedRuntime.identity(() => props.methodCall); + const call = identity(() => props.call); + const builtinElementAttr =
props.builtinElementAttr} />; + const namedElementAttr = props.namedElementAttr} />; + const hookArgument = useIdentity(() => props.hookArgument); + useEffect(() => { + console.log(props.useEffect); + JSON.stringify(null, null, () => props.useEffect); + const g = () => props.useEffect; + console.log(g()); + }, [props.useEffect]); + return ( + <> + {named()} + {namedVariable()} + {methodCall()} + {call()} + {builtinElementAttr} + {namedElementAttr} + {hookArgument()} + + ); +} + +export const TODO_FIXTURE_ENTRYPOINT = { + fn: Component, + params: [ + { + named: '', + namedVariable: '', + methodCall: '', + call: '', + builtinElementAttr: '', + namedElementAttr: '', + hookArgument: '', + useEffect: '', + }, + ], +}; From eec50b17b38ac7c051804acfe46afabef9933ff4 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 9 Sep 2025 19:58:02 +0200 Subject: [PATCH 3/5] [Flight] Only use debug component info for parent stacks (#34431) --- .../react-client/src/ReactFlightClient.js | 16 ++- .../src/__tests__/ReactFlight-test.js | 125 ++++++++++++++++-- 2 files changed, 123 insertions(+), 18 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index c7cb641cd242b..132747e60a883 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -1794,13 +1794,21 @@ function transferReferencedDebugInfo( existingDebugInfo.push.apply(existingDebugInfo, referencedDebugInfo); } } - // We also add it to the initializing chunk since the resolution of that promise is - // also blocked by these. By adding it to both we can track it even if the array/element + // We also add the debug info to the initializing chunk since the resolution of that promise is + // also blocked by the referenced debug info. By adding it to both we can track it even if the array/element // is extracted, or if the root is rendered as is. if (parentChunk !== null) { const parentDebugInfo = parentChunk._debugInfo; - // $FlowFixMe[method-unbinding] - parentDebugInfo.push.apply(parentDebugInfo, referencedDebugInfo); + for (let i = 0; i < referencedDebugInfo.length; ++i) { + const debugInfoEntry = referencedDebugInfo[i]; + if (debugInfoEntry.name != null) { + (debugInfoEntry: ReactComponentInfo); + // We're not transferring Component info since we use Component info + // in Debug info to fill in gaps between Fibers for the parent stack. + } else { + parentDebugInfo.push(debugInfoEntry); + } + } } } } diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 01f37319bf574..3bcf4988feb86 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -2960,13 +2960,6 @@ describe('ReactFlight', () => { { time: 16, }, - { - env: 'third-party', - key: null, - name: 'ThirdPartyAsyncIterableComponent', - props: {}, - stack: ' in Object. (at **)', - }, { time: 16, }, @@ -2995,13 +2988,6 @@ describe('ReactFlight', () => { { time: 19, }, - { - env: 'third-party', - key: null, - name: 'ThirdPartyAsyncIterableComponent', - props: {}, - stack: ' in Object. (at **)', - }, {time: 19}, ] : undefined, @@ -3847,4 +3833,115 @@ describe('ReactFlight', () => { expect(ReactNoop).toMatchRenderedOutput(
not using props
); }); + + // @gate !__DEV__ || enableComponentPerformanceTrack + it('produces correct parent stacks', async () => { + function Container() { + return ReactServer.createElement('div', null); + } + function ContainerParent() { + return ReactServer.createElement(Container, null); + } + function App() { + return ReactServer.createElement( + 'main', + null, + ReactServer.createElement(ContainerParent, null), + ); + } + + const transport = ReactNoopFlightServer.render({ + root: ReactServer.createElement(App, null), + }); + + await act(async () => { + const {root} = await ReactNoopFlightClient.read(transport); + + ReactNoop.render(root); + + expect(root.type).toBe('main'); + if (__DEV__) { + const div = root.props.children; + expect(getDebugInfo(div)).toEqual([ + { + time: 14, + }, + { + env: 'Server', + key: null, + name: 'ContainerParent', + owner: { + env: 'Server', + key: null, + name: 'App', + props: {}, + stack: ' in Object. (at **)', + }, + props: {}, + stack: ' in App (at **)', + }, + { + time: 15, + }, + { + env: 'Server', + key: null, + name: 'Container', + owner: { + env: 'Server', + key: null, + name: 'ContainerParent', + owner: { + env: 'Server', + key: null, + name: 'App', + props: {}, + stack: ' in Object. (at **)', + }, + props: {}, + stack: ' in App (at **)', + }, + props: {}, + stack: ' in ContainerParent (at **)', + }, + { + time: 16, + }, + ]); + expect(getDebugInfo(root)).toEqual([ + { + time: 12, + }, + { + env: 'Server', + key: null, + name: 'App', + props: {}, + stack: ' in Object. (at **)', + }, + { + time: 13, + }, + { + time: 14, + }, + { + time: 15, + }, + { + time: 16, + }, + ]); + } else { + expect(root._debugInfo).toBe(undefined); + expect(root._owner).toBe(undefined); + } + }); + + expect(ReactNoop).toMatchRenderedOutput( +
+
+
, + ); + }); }); From 1836b46fffa9a67be3840ff385f6fd1a1f8db045 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes <57368278+jorge-cab@users.noreply.github.com> Date: Tue, 9 Sep 2025 11:18:37 -0700 Subject: [PATCH 4/5] [compiler] Have react-compiler eslint plugin return a RuleModule (#34421) Eslint is expecting a map of [string] => RuleModule. Before we were passing {rule: RuleModule, severity: ErrorSeverity} which was breaking legacy Eslint configurations --- compiler/packages/eslint-plugin-react-compiler/src/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/packages/eslint-plugin-react-compiler/src/index.ts b/compiler/packages/eslint-plugin-react-compiler/src/index.ts index 9d49b16c57e8f..3f0c7bcdcb058 100644 --- a/compiler/packages/eslint-plugin-react-compiler/src/index.ts +++ b/compiler/packages/eslint-plugin-react-compiler/src/index.ts @@ -34,4 +34,8 @@ const configs = { }, }; -export {configs, allRules as rules, meta}; +const rules = Object.fromEntries( + Object.entries(allRules).map(([name, {rule}]) => [name, rule]), +); + +export {configs, rules, meta}; From eda778b8ae1698fec5fc84ca2530727df8b557b5 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Tue, 9 Sep 2025 14:26:52 -0400 Subject: [PATCH 5/5] [compiler] Fix false positive memo validation (alternative) (#34319) Alternative to #34276 --- (Summary taken from @josephsavona 's #34276) Partial fix for #34262. Consider this example: ```js function useInputValue(input) { const object = React.useMemo(() => { const {value} = transform(input); return {value}; }, [input]); return object; } ``` React Compiler breaks this code into two reactive scopes: * One for `transform(input)` * One for `{value}` When we run ValidatePreserveExistingMemo, we see that the scope for `{value}` has the dependency `value`, whereas the original memoization had the dependency `input`, and throw an error that the dependencies didn't match. In other words, we're flagging the fact that memoized _better than the user_ as a problem. The more complete solution would be to validate that there is a subgraph of reactive scopes with a single input and output node, where the input node has the same dependencies as the original useMemo, and the output has the same outputs. That is true in this case, with the subgraph being the two consecutive scopes mentioned above. But that's complicated. As a shortcut, this PR checks for any dependencies that are defined after the start of the original useMemo. If we find one, we know that it's a case where we were able to memoize more precisely than the original, and we don't report an error on the dependency. We still check that the original _output_ value is able to be memoized, though. So if the scope of `object` were extended, eg with a call to `mutate(object)`, then we'd still correctly report an error that we couldn't preserve memoization. Co-authored-by: Joe Savona --- .../ValidatePreservedManualMemoization.ts | 85 +++++++------- ...ken-as-dependency-later-mutation.expect.md | 59 ++++++++++ ...e-mistaken-as-dependency-later-mutation.js | 25 ++++ ...staken-as-dependency-mutated-dep.expect.md | 64 ++++++++++ ...alue-mistaken-as-dependency-mutated-dep.js | 36 ++++++ ...red-value-mistaken-as-dependency.expect.md | 109 ++++++++++++++++++ ...structured-value-mistaken-as-dependency.js | 31 +++++ 7 files changed, 364 insertions(+), 45 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts index 49d8a85a96b3c..19af0ed080142 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -18,7 +18,6 @@ import { IdentifierId, InstructionValue, ManualMemoDependency, - Place, PrunedReactiveScopeBlock, ReactiveFunction, ReactiveInstruction, @@ -29,7 +28,10 @@ import { SourceLocation, } from '../HIR'; import {printIdentifier, printManualMemoDependency} from '../HIR/PrintHIR'; -import {eachInstructionValueOperand} from '../HIR/visitors'; +import { + eachInstructionValueLValue, + eachInstructionValueOperand, +} from '../HIR/visitors'; import {collectMaybeMemoDependencies} from '../Inference/DropManualMemoization'; import { ReactiveFunctionVisitor, @@ -337,56 +339,53 @@ class Visitor extends ReactiveFunctionVisitor { * @returns a @{ManualMemoDependency} representing the variable + * property reads represented by @value */ - recordDepsInValue( - value: ReactiveValue, - state: VisitorState, - ): ManualMemoDependency | null { + recordDepsInValue(value: ReactiveValue, state: VisitorState): void { switch (value.kind) { case 'SequenceExpression': { for (const instr of value.instructions) { this.visitInstruction(instr, state); } - const result = this.recordDepsInValue(value.value, state); - return result; + this.recordDepsInValue(value.value, state); + break; } case 'OptionalExpression': { - return this.recordDepsInValue(value.value, state); + this.recordDepsInValue(value.value, state); + break; } case 'ConditionalExpression': { this.recordDepsInValue(value.test, state); this.recordDepsInValue(value.consequent, state); this.recordDepsInValue(value.alternate, state); - return null; + break; } case 'LogicalExpression': { this.recordDepsInValue(value.left, state); this.recordDepsInValue(value.right, state); - return null; + break; } default: { - const dep = collectMaybeMemoDependencies( - value, - this.temporaries, - false, - ); - if (value.kind === 'StoreLocal' || value.kind === 'StoreContext') { - const storeTarget = value.lvalue.place; - state.manualMemoState?.decls.add( - storeTarget.identifier.declarationId, - ); - if (storeTarget.identifier.name?.kind === 'named' && dep == null) { - const dep: ManualMemoDependency = { - root: { - kind: 'NamedLocal', - value: storeTarget, - }, - path: [], - }; - this.temporaries.set(storeTarget.identifier.id, dep); - return dep; + collectMaybeMemoDependencies(value, this.temporaries, false); + if ( + value.kind === 'StoreLocal' || + value.kind === 'StoreContext' || + value.kind === 'Destructure' + ) { + for (const storeTarget of eachInstructionValueLValue(value)) { + state.manualMemoState?.decls.add( + storeTarget.identifier.declarationId, + ); + if (storeTarget.identifier.name?.kind === 'named') { + this.temporaries.set(storeTarget.identifier.id, { + root: { + kind: 'NamedLocal', + value: storeTarget, + }, + path: [], + }); + } } } - return dep; + break; } } } @@ -403,19 +402,15 @@ class Visitor extends ReactiveFunctionVisitor { state.manualMemoState.decls.add(lvalue.identifier.declarationId); } - const maybeDep = this.recordDepsInValue(value, state); - if (lvalId != null) { - if (maybeDep != null) { - temporaries.set(lvalId, maybeDep); - } else if (isNamedLocal) { - temporaries.set(lvalId, { - root: { - kind: 'NamedLocal', - value: {...(instr.lvalue as Place)}, - }, - path: [], - }); - } + this.recordDepsInValue(value, state); + if (lvalue != null) { + temporaries.set(lvalue.identifier.id, { + root: { + kind: 'NamedLocal', + value: {...lvalue}, + }, + path: [], + }); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.expect.md new file mode 100644 index 0000000000000..d8223e2949837 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.expect.md @@ -0,0 +1,59 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +/** + * Repro from https://github.com/facebook/react/issues/34262 + * + * The compiler memoizes more precisely than the original code, with two reactive scopes: + * - One for `transform(input)` with `input` as dep + * - One for `{value}` with `value` as dep + * + * When we validate preserving manual memoization we incorrectly reject this, because + * the original memoization had `object` depending on `input` but our scope depends on + * `value`. + * + * This fixture adds a later potential mutation, which extends the scope and should + * fail validation. This confirms that even though we allow the dependency to diverge, + * we still check that the output value is memoized. + */ +function useInputValue(input) { + const object = React.useMemo(() => { + const {value} = transform(input); + return {value}; + }, [input]); + mutate(object); + return object; +} + +``` + + +## 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. This value was memoized in source but not in compilation output. + +error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.ts:19:17 + 17 | */ + 18 | function useInputValue(input) { +> 19 | const object = React.useMemo(() => { + | ^^^^^^^^^^^^^^^^^^^^^ +> 20 | const {value} = transform(input); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +> 21 | return {value}; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +> 22 | }, [input]); + | ^^^^^^^^^^^^^^ Could not preserve existing memoization + 23 | mutate(object); + 24 | return object; + 25 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.js new file mode 100644 index 0000000000000..f07d00854cd85 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-later-mutation.js @@ -0,0 +1,25 @@ +// @validatePreserveExistingMemoizationGuarantees + +/** + * Repro from https://github.com/facebook/react/issues/34262 + * + * The compiler memoizes more precisely than the original code, with two reactive scopes: + * - One for `transform(input)` with `input` as dep + * - One for `{value}` with `value` as dep + * + * When we validate preserving manual memoization we incorrectly reject this, because + * the original memoization had `object` depending on `input` but our scope depends on + * `value`. + * + * This fixture adds a later potential mutation, which extends the scope and should + * fail validation. This confirms that even though we allow the dependency to diverge, + * we still check that the output value is memoized. + */ +function useInputValue(input) { + const object = React.useMemo(() => { + const {value} = transform(input); + return {value}; + }, [input]); + mutate(object); + return object; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.expect.md new file mode 100644 index 0000000000000..371348a560643 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.expect.md @@ -0,0 +1,64 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +import {identity, Stringify, useHook} from 'shared-runtime'; + +/** + * Repro from https://github.com/facebook/react/issues/34262 + * + * The compiler memoizes more precisely than the original code, with two reactive scopes: + * - One for `transform(input)` with `input` as dep + * - One for `{value}` with `value` as dep + * + * When we validate preserving manual memoization we incorrectly reject this, because + * the original memoization had `object` depending on `input` but our scope depends on + * `value`. + */ +function useInputValue(input) { + // Conflate the `identity(input, x)` call with something outside the useMemo, + // to try and break memoization of `value`. This gets correctly flagged since + // the dependency is being mutated + let x = {}; + useHook(); + const object = React.useMemo(() => { + const {value} = identity(input, x); + return {value}; + }, [input, x]); + return object; +} + +function Component() { + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + + +## 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. This dependency may be mutated later, which could cause the value to change unexpectedly. + +error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.ts:25:13 + 23 | const {value} = identity(input, x); + 24 | return {value}; +> 25 | }, [input, x]); + | ^ This dependency may be modified later + 26 | return object; + 27 | } + 28 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.js new file mode 100644 index 0000000000000..06d2945868dd1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency-mutated-dep.js @@ -0,0 +1,36 @@ +// @validatePreserveExistingMemoizationGuarantees + +import {identity, Stringify, useHook} from 'shared-runtime'; + +/** + * Repro from https://github.com/facebook/react/issues/34262 + * + * The compiler memoizes more precisely than the original code, with two reactive scopes: + * - One for `transform(input)` with `input` as dep + * - One for `{value}` with `value` as dep + * + * When we validate preserving manual memoization we incorrectly reject this, because + * the original memoization had `object` depending on `input` but our scope depends on + * `value`. + */ +function useInputValue(input) { + // Conflate the `identity(input, x)` call with something outside the useMemo, + // to try and break memoization of `value`. This gets correctly flagged since + // the dependency is being mutated + let x = {}; + useHook(); + const object = React.useMemo(() => { + const {value} = identity(input, x); + return {value}; + }, [input, x]); + return object; +} + +function Component() { + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.expect.md new file mode 100644 index 0000000000000..eca36e2a7919d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.expect.md @@ -0,0 +1,109 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees + +import {identity, Stringify} from 'shared-runtime'; + +/** + * Repro from https://github.com/facebook/react/issues/34262 + * + * The compiler memoizes more precisely than the original code, with two reactive scopes: + * - One for `transform(input)` with `input` as dep + * - One for `{value}` with `value` as dep + * + * Previously ValidatePreservedManualMemoization rejected this input, because + * the original memoization had `object` depending on `input` but we split the scope per above, + * and the scope for the FinishMemoize instruction is the second scope which depends on `value` + */ +function useInputValue(input) { + const object = React.useMemo(() => { + const {value} = identity(input); + return {value}; + }, [input]); + return object; +} + +function Component() { + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees + +import { identity, Stringify } from "shared-runtime"; + +/** + * Repro from https://github.com/facebook/react/issues/34262 + * + * The compiler memoizes more precisely than the original code, with two reactive scopes: + * - One for `transform(input)` with `input` as dep + * - One for `{value}` with `value` as dep + * + * Previously ValidatePreservedManualMemoization rejected this input, because + * the original memoization had `object` depending on `input` but we split the scope per above, + * and the scope for the FinishMemoize instruction is the second scope which depends on `value` + */ +function useInputValue(input) { + const $ = _c(4); + let t0; + if ($[0] !== input) { + t0 = identity(input); + $[0] = input; + $[1] = t0; + } else { + t0 = $[1]; + } + const { value } = t0; + let t1; + if ($[2] !== value) { + t1 = { value }; + $[2] = value; + $[3] = t1; + } else { + t1 = $[3]; + } + const object = t1; + return object; +} + +function Component() { + const $ = _c(3); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = { value: 42 }; + $[0] = t0; + } else { + t0 = $[0]; + } + const t1 = useInputValue(t0); + let t2; + if ($[1] !== t1.value) { + t2 = ; + $[1] = t1.value; + $[2] = t2; + } else { + t2 = $[2]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
{"value":42}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.js new file mode 100644 index 0000000000000..a9533ce44bbde --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-preserve-memoization-inner-destructured-value-mistaken-as-dependency.js @@ -0,0 +1,31 @@ +// @validatePreserveExistingMemoizationGuarantees + +import {identity, Stringify} from 'shared-runtime'; + +/** + * Repro from https://github.com/facebook/react/issues/34262 + * + * The compiler memoizes more precisely than the original code, with two reactive scopes: + * - One for `transform(input)` with `input` as dep + * - One for `{value}` with `value` as dep + * + * Previously ValidatePreservedManualMemoization rejected this input, because + * the original memoization had `object` depending on `input` but we split the scope per above, + * and the scope for the FinishMemoize instruction is the second scope which depends on `value` + */ +function useInputValue(input) { + const object = React.useMemo(() => { + const {value} = identity(input); + return {value}; + }, [input]); + return object; +} + +function Component() { + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +};