From 33a0fbe3fe4a2bb96abb07de490df14e7e9b96fa Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 9 Sep 2025 10:36:48 -0700 Subject: [PATCH] [compiler] Improve name hints for outlined functions The previous PR added name hints for anonymous functions, but didn't handle the case of outlined functions. Here we do some cleanup around function `id` and name hints: * Make `HIRFunction.id` a ValidatedIdentifierName, which involved some cleanup of the validation helpers * Add `HIRFunction.nameHint: string` as a place to store the generated name hints which are not valid identifiers * Update Codegen to always use the `id` as the actual function name, and only use nameHint as part of generating the object+property wrapper for debug purposes. This ensures we don't conflate synthesized hints with real function names. Then, we also update OutlineFunctions to use the function name _or_ the nameHint as the input to generating a unique identifier. This isn't quite as nice as the object form since we lose our formatting, but it's a simple step that gives more context to the developer than `_temp` does. Switching to output the object+property lookup form for outlined functions is a bit more involved, let's do that in a follow-up. --- .../src/Entrypoint/Pipeline.ts | 18 ++--- .../src/HIR/BuildHIR.ts | 23 ++++-- .../src/HIR/HIR.ts | 79 +++++++++++-------- .../src/HIR/PrintHIR.ts | 3 + .../src/Optimization/LowerContextAccess.ts | 2 + .../src/Optimization/OutlineFunctions.ts | 4 +- .../src/Optimization/OutlineJsx.ts | 1 + .../ReactiveScopes/BuildReactiveFunction.ts | 1 + .../ReactiveScopes/CodegenReactiveFunction.ts | 17 ++-- .../src/Transform/NameAnonymousFunctions.ts | 3 +- ...name-anonymous-functions-outline.expect.md | 52 ++++++++++++ .../name-anonymous-functions-outline.js | 14 ++++ 12 files changed, 154 insertions(+), 63 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.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 7f8640122b94..e0b0536f28cc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -325,6 +325,15 @@ function runWithEnvironment( outlineJSX(hir); } + if (env.config.enableNameAnonymousFunctions) { + nameAnonymousFunctions(hir); + log({ + kind: 'hir', + name: 'NameAnonymousFunctions', + value: hir, + }); + } + if (env.config.enableFunctionOutlining) { outlineFunctions(hir, fbtOperands); log({kind: 'hir', name: 'OutlineFunctions', value: hir}); @@ -415,15 +424,6 @@ 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 4d0461fe105b..702eaf0f692a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -47,6 +47,7 @@ import { makePropertyLiteral, makeType, promoteTemporary, + validateIdentifierName, } from './HIR'; import HIRBuilder, {Bindings, createTemporaryPlace} from './HIRBuilder'; import {BuiltInArrayId} from './ObjectShape'; @@ -213,6 +214,16 @@ export function lower( ); } + let validatedId: HIRFunction['id'] = null; + if (id != null) { + const idResult = validateIdentifierName(id); + if (idResult.isErr()) { + builder.errors.merge(idResult.unwrapErr()); + } else { + validatedId = idResult.unwrap().value; + } + } + if (builder.errors.hasAnyErrors()) { return Err(builder.errors); } @@ -234,7 +245,8 @@ export function lower( ); return Ok({ - id, + id: validatedId, + nameHint: null, params, fnType: bindings == null ? env.fnType : 'Other', returnTypeAnnotation: null, // TODO: extract the actual return type node if present @@ -3563,19 +3575,14 @@ function lowerFunctionToValue( ): InstructionValue { const exprNode = expr.node; const exprLoc = exprNode.loc ?? GeneratedSource; - 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) { return {kind: 'UnsupportedNode', node: exprNode, loc: exprLoc}; } return { kind: 'FunctionExpression', - name, + name: loweredFunc.func.id, + nameHint: null, type: expr.node.type, loc: exprLoc, loweredFunc, 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 fa502f821d0b..4d2d4ed80d69 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -7,7 +7,11 @@ import {BindingKind} from '@babel/traverse'; import * as t from '@babel/types'; -import {CompilerError} from '../CompilerError'; +import { + CompilerDiagnostic, + CompilerError, + ErrorCategory, +} from '../CompilerError'; import {assertExhaustive} from '../Utils/utils'; import {Environment, ReactFunctionType} from './Environment'; import type {HookKind} from './ObjectShape'; @@ -54,7 +58,8 @@ export type SourceLocation = t.SourceLocation | typeof GeneratedSource; */ export type ReactiveFunction = { loc: SourceLocation; - id: string | null; + id: ValidIdentifierName | null; + nameHint: string | null; params: Array; generator: boolean; async: boolean; @@ -276,7 +281,8 @@ export type ReactiveTryTerminal = { // A function lowered to HIR form, ie where its body is lowered to an HIR control-flow graph export type HIRFunction = { loc: SourceLocation; - id: string | null; + id: ValidIdentifierName | null; + nameHint: string | null; fnType: ReactFunctionType; env: Environment; params: Array; @@ -1124,7 +1130,8 @@ export type JsxAttribute = export type FunctionExpression = { kind: 'FunctionExpression'; - name: string | null; + name: ValidIdentifierName | null; + nameHint: string | null; loweredFunc: LoweredFunction; type: | 'ArrowFunctionExpression' @@ -1301,11 +1308,41 @@ export function forkTemporaryIdentifier( export function validateIdentifierName( name: string, -): Result { - if (isReservedWord(name) || !t.isValidIdentifier(name)) { - return Err(null); +): Result { + if (isReservedWord(name)) { + const error = new CompilerError(); + error.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.Syntax, + reason: 'Expected a non-reserved identifier name', + description: `\`${name}\` is a reserved word in JavaScript and cannot be used as an identifier name`, + suggestions: null, + }).withDetails({ + kind: 'error', + loc: GeneratedSource, + message: 'reserved word', + }), + ); + return Err(error); + } else if (!t.isValidIdentifier(name)) { + const error = new CompilerError(); + error.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.Syntax, + reason: `Expected a valid identifier name`, + description: `\`${name}\` is not a valid JavaScript identifier`, + suggestions: null, + }).withDetails({ + kind: 'error', + loc: GeneratedSource, + message: 'reserved word', + }), + ); } - return Ok(makeIdentifierName(name).value); + return Ok({ + kind: 'named', + value: name as ValidIdentifierName, + }); } /** @@ -1314,31 +1351,7 @@ export function validateIdentifierName( * original source code. */ export function makeIdentifierName(name: string): ValidatedIdentifier { - if (isReservedWord(name)) { - CompilerError.throwInvalidJS({ - reason: 'Expected a non-reserved identifier name', - loc: GeneratedSource, - description: `\`${name}\` is a reserved word in JavaScript and cannot be used as an identifier name`, - suggestions: null, - }); - } else { - CompilerError.invariant(t.isValidIdentifier(name), { - reason: `Expected a valid identifier name`, - description: `\`${name}\` is not a valid JavaScript identifier`, - details: [ - { - kind: 'error', - loc: GeneratedSource, - message: null, - }, - ], - suggestions: null, - }); - } - return { - kind: 'named', - value: name as ValidIdentifierName, - }; + return validateIdentifierName(name).unwrap(); } /** diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 197e486e8af3..71fb4c43b33e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -56,6 +56,9 @@ export function printFunction(fn: HIRFunction): string { } else { definition += '<>'; } + if (fn.nameHint != null) { + definition += ` ${fn.nameHint}`; + } if (fn.params.length !== 0) { definition += '(' + diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts index 62845934c16f..50f00427205b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts @@ -249,6 +249,7 @@ function emitSelectorFn(env: Environment, keys: Array): Instruction { const fn: HIRFunction = { loc: GeneratedSource, id: null, + nameHint: null, fnType: 'Other', env, params: [obj], @@ -275,6 +276,7 @@ function emitSelectorFn(env: Environment, keys: Array): Instruction { value: { kind: 'FunctionExpression', name: null, + nameHint: null, loweredFunc: { func: fn, }, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts index 0e6d1fd59201..6ab0a811ff5d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts @@ -31,7 +31,9 @@ export function outlineFunctions( ) { const loweredFunc = value.loweredFunc.func; - const id = fn.env.generateGloballyUniqueIdentifierName(loweredFunc.id); + const id = fn.env.generateGloballyUniqueIdentifierName( + loweredFunc.id ?? loweredFunc.nameHint, + ); loweredFunc.id = id.value; fn.env.outlineFunction(loweredFunc, null); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts index 6232456e620c..5c98997953d0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts @@ -364,6 +364,7 @@ function emitOutlinedFn( const fn: HIRFunction = { loc: GeneratedSource, id: null, + nameHint: null, fnType: 'Other', env, params: [propsObj], diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts index b400bb349876..2e8584050eb7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts @@ -44,6 +44,7 @@ export function buildReactiveFunction(fn: HIRFunction): ReactiveFunction { return { loc: fn.loc, id: fn.id, + nameHint: fn.nameHint, params: fn.params, generator: fn.generator, async: fn.async, 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 28d8afd84b4f..c497253a22fb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -43,7 +43,6 @@ import { ValidIdentifierName, getHookKind, makeIdentifierName, - validateIdentifierName, } from '../HIR/HIR'; import {printIdentifier, printInstruction, printPlace} from '../HIR/PrintHIR'; import {eachPatternOperand} from '../HIR/visitors'; @@ -62,6 +61,7 @@ export const EARLY_RETURN_SENTINEL = 'react.early_return_sentinel'; export type CodegenFunction = { type: 'CodegenFunction'; id: t.Identifier | null; + nameHint: string | null; params: t.FunctionDeclaration['params']; body: t.BlockStatement; generator: boolean; @@ -384,6 +384,7 @@ function codegenReactiveFunction( type: 'CodegenFunction', loc: fn.loc, id: fn.id !== null ? t.identifier(fn.id) : null, + nameHint: fn.nameHint, params, body, generator: fn.generator, @@ -2328,10 +2329,6 @@ 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) { @@ -2343,9 +2340,7 @@ function codegenInstructionValue( value = t.arrowFunctionExpression(fn.params, body, fn.async); } else { value = t.functionExpression( - validatedName - .map(name => t.identifier(name)) - .unwrapOr(null), + instrValue.name != null ? t.identifier(instrValue.name) : null, fn.params, fn.body, fn.generator, @@ -2354,10 +2349,10 @@ function codegenInstructionValue( } if ( cx.env.config.enableNameAnonymousFunctions && - validatedName.isErr() && - instrValue.name != null + instrValue.name == null && + instrValue.nameHint != null ) { - const name = instrValue.name; + const name = instrValue.nameHint; value = t.memberExpression( t.objectExpression([t.objectProperty(t.stringLiteral(name), value)]), t.stringLiteral(name), diff --git a/compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts index 5fed3af0f733..cf6d443b907a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts @@ -26,7 +26,8 @@ export function nameAnonymousFunctions(fn: HIRFunction): void { * nesting depth. */ const name = `${prefix}${node.generatedName}]`; - node.fn.name = name; + node.fn.nameHint = name; + node.fn.loweredFunc.func.nameHint = name; } /** * Whether or not we generated a name for the function at this node, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.expect.md new file mode 100644 index 000000000000..a1267c6f8295 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.expect.md @@ -0,0 +1,52 @@ + +## Input + +```javascript +// @enableNameAnonymousFunctions +import {Stringify} from 'shared-runtime'; + +function Component(props) { + const onClick = () => { + console.log('hello!'); + }; + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNameAnonymousFunctions +import { Stringify } from "shared-runtime"; + +function Component(props) { + const $ = _c(1); + const onClick = _ComponentOnClick; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} +function _ComponentOnClick() { + console.log("hello!"); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 42 }], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.js new file mode 100644 index 000000000000..0906cb928a8c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/name-anonymous-functions-outline.js @@ -0,0 +1,14 @@ +// @enableNameAnonymousFunctions +import {Stringify} from 'shared-runtime'; + +function Component(props) { + const onClick = () => { + console.log('hello!'); + }; + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], +};