From adbc32de32bc52f9014cedb5ff5a502be35aff51 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 17 Oct 2025 11:37:28 -0700 Subject: [PATCH] [compiler] More fbt compatibility (#34887) In my previous PR I fixed some cases but broke others. So, new approach. Two phase algorithm: * First pass is forward data flow to determine all usages of macros. This is necessary because many of Meta's macros have variants that can be accessed via properties, eg you can do `macro(...)` but also `macro.variant(...)`. * Second pass is backwards data flow to find macro invocations (JSX and calls) and then merge their operands into the same scope as the macro call. Note that this required updating PromoteUsedTemporaries to avoid promoting macro calls that have interposing instructions between their creation and usage. Macro calls in general are pure so it should be safe to reorder them. In addition, we're now more precise about ``, ``, `fbt.plural()` and `fbt.param()`, which don't actually require all their arguments to be inlined. The whole point is that the plural/param value is an arbitrary value (along with a string name). So we no longer transitively inline the arguments, we just make sure that they don't get inadvertently promoted to named variables. One caveat: we actually don't do anything to treat macro functions as non-mutating, so `fbt.plural()` and friends (function form) may still sometimes group arguments just due to mutability inference. In a follow-up, i'll work to infer the types of nested macro functions as non-mutating. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34887). * #34900 * __->__ #34887 --- .../src/HIR/Environment.ts | 12 +- .../MemoizeFbtAndMacroOperandsInSameScope.ts | 432 +++++++++--------- .../src/Utils/TestUtils.ts | 11 +- ...bt-param-with-leading-whitespace.expect.md | 12 +- ...t-param-with-trailing-whitespace.expect.md | 12 +- ...preserve-whitespace-two-subtrees.expect.md | 35 +- ...btparam-with-jsx-element-content.expect.md | 16 +- ...fbtparam-with-jsx-fragment-value.expect.md | 17 +- .../recursively-merge-scopes-jsx.expect.md | 109 +++++ .../fbt/recursively-merge-scopes-jsx.js | 35 ++ .../repro-fbt-param-nested-fbt-jsx.expect.md | 128 ++++++ .../fbt/repro-fbt-param-nested-fbt-jsx.js | 42 ++ .../fbt/repro-fbt-param-nested-fbt.expect.md | 41 +- .../fbt/repro-fbt-param-nested-fbt.js | 17 +- ...idx-method-no-outlining-wildcard.expect.md | 10 +- .../idx-method-no-outlining.expect.md | 5 +- 16 files changed, 640 insertions(+), 294 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.js 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 4ec4a4a795f1b..bdbfb20a59ac1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -83,21 +83,11 @@ export type ExternalFunction = z.infer; export const USE_FIRE_FUNCTION_NAME = 'useFire'; export const EMIT_FREEZE_GLOBAL_GATING = '__DEV__'; -export const MacroMethodSchema = z.union([ - z.object({type: z.literal('wildcard')}), - z.object({type: z.literal('name'), name: z.string()}), -]); - -// Would like to change this to drop the string option, but breaks compatibility with existing configs -export const MacroSchema = z.union([ - z.string(), - z.tuple([z.string(), z.array(MacroMethodSchema)]), -]); +export const MacroSchema = z.string(); export type CompilerMode = 'all_features' | 'no_inferred_memo'; export type Macro = z.infer; -export type MacroMethod = z.infer; const HookSchema = z.object({ /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts index 4ae9978b589f7..0ce05a823e9b0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts @@ -7,7 +7,6 @@ import { HIRFunction, - Identifier, IdentifierId, InstructionValue, makeInstructionId, @@ -15,9 +14,35 @@ import { Place, ReactiveScope, } from '../HIR'; -import {Macro, MacroMethod} from '../HIR/Environment'; +import {Macro} from '../HIR/Environment'; import {eachInstructionValueOperand} from '../HIR/visitors'; -import {Iterable_some} from '../Utils/utils'; + +/** + * Whether a macro requires its arguments to be transitively inlined (eg fbt) + * or just avoid having the top-level values be converted to variables (eg fbt.param) + */ +enum InlineLevel { + Transitive = 'Transitive', + Shallow = 'Shallow', +} +type MacroDefinition = { + level: InlineLevel; + properties: Map | null; +}; + +const SHALLOW_MACRO: MacroDefinition = { + level: InlineLevel.Shallow, + properties: null, +}; +const TRANSITIVE_MACRO: MacroDefinition = { + level: InlineLevel.Transitive, + properties: null, +}; +const FBT_MACRO: MacroDefinition = { + level: InlineLevel.Transitive, + properties: new Map([['*', SHALLOW_MACRO]]), +}; +FBT_MACRO.properties!.set('enum', FBT_MACRO); /** * This pass supports the `fbt` translation system (https://facebook.github.io/fbt/) @@ -42,250 +67,210 @@ import {Iterable_some} from '../Utils/utils'; * ## User-defined macro-like function * * Users can also specify their own functions to be treated similarly to fbt via the - * `customMacros` environment configuration. + * `customMacros` environment configuration. By default, user-supplied custom macros + * have their arguments transitively inlined. */ export function memoizeFbtAndMacroOperandsInSameScope( fn: HIRFunction, ): Set { - const fbtMacroTags = new Set([ - ...Array.from(FBT_TAGS).map((tag): Macro => [tag, []]), - ...(fn.env.config.customMacros ?? []), + const macroKinds = new Map([ + ...Array.from(FBT_TAGS.entries()), + ...(fn.env.config.customMacros ?? []).map( + name => [name, TRANSITIVE_MACRO] as [Macro, MacroDefinition], + ), ]); /** - * Set of all identifiers that load fbt or other macro functions or their nested - * properties, as well as values known to be the results of invoking macros + * Forward data-flow analysis to identify all macro tags, including + * things like `fbt.foo.bar(...)` */ - const macroTagsCalls: Set = new Set(); + const macroTags = populateMacroTags(fn, macroKinds); + /** - * Mapping of lvalue => list of operands for all expressions where either - * the lvalue is a known fbt/macro call and/or the operands transitively - * contain fbt/macro calls. - * - * This is the key data structure that powers the scope merging: we start - * at the lvalues and merge operands into the lvalue's scope. + * Reverse data-flow analysis to merge arguments to macro *invocations* + * based on the kind of the macro */ - const macroValues: Map> = new Map(); - // Tracks methods loaded from macros, like fbt.param or idx.foo - const macroMethods = new Map>>(); - - visit(fn, fbtMacroTags, macroTagsCalls, macroMethods, macroValues); - - for (const root of macroValues.keys()) { - const scope = root.scope; - if (scope == null) { - continue; - } - // Merge the operands into the same scope if this is a known macro invocation - if (!macroTagsCalls.has(root.id)) { - continue; - } - mergeScopes(root, scope, macroValues, macroTagsCalls); - } + const macroValues = mergeMacroArguments(fn, macroTags, macroKinds); - return macroTagsCalls; + return macroValues; } -export const FBT_TAGS: Set = new Set([ - 'fbt', - 'fbt:param', - 'fbt:enum', - 'fbt:plural', - 'fbs', - 'fbs:param', - 'fbs:enum', - 'fbs:plural', +const FBT_TAGS: Map = new Map([ + ['fbt', FBT_MACRO], + ['fbt:param', SHALLOW_MACRO], + ['fbt:enum', FBT_MACRO], + ['fbt:plural', SHALLOW_MACRO], + ['fbs', FBT_MACRO], + ['fbs:param', SHALLOW_MACRO], + ['fbs:enum', FBT_MACRO], + ['fbs:plural', SHALLOW_MACRO], ]); export const SINGLE_CHILD_FBT_TAGS: Set = new Set([ 'fbt:param', 'fbs:param', ]); -function visit( +function populateMacroTags( fn: HIRFunction, - fbtMacroTags: Set, - macroTagsCalls: Set, - macroMethods: Map>>, - macroValues: Map>, -): void { - for (const [, block] of fn.body.blocks) { - for (const phi of block.phis) { - const macroOperands: Array = []; - for (const operand of phi.operands.values()) { - if (macroValues.has(operand.identifier)) { - macroOperands.push(operand.identifier); - } - } - if (macroOperands.length !== 0) { - macroValues.set(phi.place.identifier, macroOperands); - } - } - for (const instruction of block.instructions) { - const {lvalue, value} = instruction; - if (lvalue === null) { - continue; - } - if ( - value.kind === 'Primitive' && - typeof value.value === 'string' && - matchesExactTag(value.value, fbtMacroTags) - ) { - /* - * We don't distinguish between tag names and strings, so record - * all `fbt` string literals in case they are used as a jsx tag. - */ - macroTagsCalls.add(lvalue.identifier.id); - } else if ( - value.kind === 'LoadGlobal' && - matchesExactTag(value.binding.name, fbtMacroTags) - ) { - // Record references to `fbt` as a global - macroTagsCalls.add(lvalue.identifier.id); - } else if ( - value.kind === 'LoadGlobal' && - matchTagRoot(value.binding.name, fbtMacroTags) !== null - ) { - const methods = matchTagRoot(value.binding.name, fbtMacroTags)!; - macroMethods.set(lvalue.identifier.id, methods); - } else if ( - value.kind === 'PropertyLoad' && - macroMethods.has(value.object.identifier.id) - ) { - const methods = macroMethods.get(value.object.identifier.id)!; - const newMethods = []; - for (const method of methods) { - if ( - method.length > 0 && - (method[0].type === 'wildcard' || - (method[0].type === 'name' && method[0].name === value.property)) - ) { - if (method.length > 1) { - newMethods.push(method.slice(1)); - } else { - macroTagsCalls.add(lvalue.identifier.id); + macroKinds: Map, +): Map { + const macroTags = new Map(); + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const {lvalue, value} = instr; + switch (value.kind) { + case 'Primitive': { + if (typeof value.value === 'string') { + const macroDefinition = macroKinds.get(value.value); + if (macroDefinition != null) { + /* + * We don't distinguish between tag names and strings, so record + * all `fbt` string literals in case they are used as a jsx tag. + */ + macroTags.set(lvalue.identifier.id, macroDefinition); } } + break; } - if (newMethods.length > 0) { - macroMethods.set(lvalue.identifier.id, newMethods); + case 'LoadGlobal': { + let macroDefinition = macroKinds.get(value.binding.name); + if (macroDefinition != null) { + macroTags.set(lvalue.identifier.id, macroDefinition); + } + break; } - } else if ( - value.kind === 'PropertyLoad' && - macroTagsCalls.has(value.object.identifier.id) - ) { - macroTagsCalls.add(lvalue.identifier.id); - } else if ( - isFbtJsxExpression(fbtMacroTags, macroTagsCalls, value) || - isFbtJsxChild(macroTagsCalls, lvalue, value) || - isFbtCallExpression(macroTagsCalls, value) - ) { - macroTagsCalls.add(lvalue.identifier.id); - macroValues.set( - lvalue.identifier, - Array.from( - eachInstructionValueOperand(value), - operand => operand.identifier, - ), - ); - } else if ( - Iterable_some(eachInstructionValueOperand(value), operand => - macroValues.has(operand.identifier), - ) - ) { - const macroOperands: Array = []; - for (const operand of eachInstructionValueOperand(value)) { - if (macroValues.has(operand.identifier)) { - macroOperands.push(operand.identifier); + case 'PropertyLoad': { + if (typeof value.property === 'string') { + const macroDefinition = macroTags.get(value.object.identifier.id); + if (macroDefinition != null) { + const propertyDefinition = + macroDefinition.properties != null + ? (macroDefinition.properties.get(value.property) ?? + macroDefinition.properties.get('*')) + : null; + const propertyMacro = propertyDefinition ?? macroDefinition; + macroTags.set(lvalue.identifier.id, propertyMacro); + } } + break; } - macroValues.set(lvalue.identifier, macroOperands); } } } + return macroTags; } -function mergeScopes( - root: Identifier, - scope: ReactiveScope, - macroValues: Map>, - macroTagsCalls: Set, -): void { - const operands = macroValues.get(root); - if (operands == null) { - return; - } - for (const operand of operands) { - operand.scope = scope; - expandFbtScopeRange(scope.range, operand.mutableRange); - macroTagsCalls.add(operand.id); - mergeScopes(operand, scope, macroValues, macroTagsCalls); - } -} - -function matchesExactTag(s: string, tags: Set): boolean { - return Array.from(tags).some(macro => - typeof macro === 'string' - ? s === macro - : macro[1].length === 0 && macro[0] === s, - ); -} - -function matchTagRoot( - s: string, - tags: Set, -): Array> | null { - const methods: Array> = []; - for (const macro of tags) { - if (typeof macro === 'string') { - continue; +function mergeMacroArguments( + fn: HIRFunction, + macroTags: Map, + macroKinds: Map, +): Set { + const macroValues = new Set(macroTags.keys()); + for (const block of Array.from(fn.body.blocks.values()).reverse()) { + for (let i = block.instructions.length - 1; i >= 0; i--) { + const instr = block.instructions[i]!; + const {lvalue, value} = instr; + switch (value.kind) { + case 'DeclareContext': + case 'DeclareLocal': + case 'Destructure': + case 'LoadContext': + case 'LoadLocal': + case 'PostfixUpdate': + case 'PrefixUpdate': + case 'StoreContext': + case 'StoreLocal': { + // Instructions that never need to be merged + break; + } + case 'CallExpression': + case 'MethodCall': { + const scope = lvalue.identifier.scope; + if (scope == null) { + continue; + } + const callee = + value.kind === 'CallExpression' ? value.callee : value.property; + const macroDefinition = + macroTags.get(callee.identifier.id) ?? + macroTags.get(lvalue.identifier.id); + if (macroDefinition != null) { + visitOperands( + macroDefinition, + scope, + lvalue, + value, + macroValues, + macroTags, + ); + } + break; + } + case 'JsxExpression': { + const scope = lvalue.identifier.scope; + if (scope == null) { + continue; + } + let macroDefinition; + if (value.tag.kind === 'Identifier') { + macroDefinition = macroTags.get(value.tag.identifier.id); + } else { + macroDefinition = macroKinds.get(value.tag.name); + } + macroDefinition ??= macroTags.get(lvalue.identifier.id); + if (macroDefinition != null) { + visitOperands( + macroDefinition, + scope, + lvalue, + value, + macroValues, + macroTags, + ); + } + break; + } + default: { + const scope = lvalue.identifier.scope; + if (scope == null) { + continue; + } + const macroDefinition = macroTags.get(lvalue.identifier.id); + if (macroDefinition != null) { + visitOperands( + macroDefinition, + scope, + lvalue, + value, + macroValues, + macroTags, + ); + } + break; + } + } } - const [tag, rest] = macro; - if (tag === s && rest.length > 0) { - methods.push(rest); + for (const phi of block.phis) { + const scope = phi.place.identifier.scope; + if (scope == null) { + continue; + } + const macroDefinition = macroTags.get(phi.place.identifier.id); + if ( + macroDefinition == null || + macroDefinition.level === InlineLevel.Shallow + ) { + continue; + } + macroValues.add(phi.place.identifier.id); + for (const operand of phi.operands.values()) { + operand.identifier.scope = scope; + expandFbtScopeRange(scope.range, operand.identifier.mutableRange); + macroTags.set(operand.identifier.id, macroDefinition); + macroValues.add(operand.identifier.id); + } } } - if (methods.length > 0) { - return methods; - } else { - return null; - } -} - -function isFbtCallExpression( - macroTagsCalls: Set, - value: InstructionValue, -): boolean { - return ( - (value.kind === 'CallExpression' && - macroTagsCalls.has(value.callee.identifier.id)) || - (value.kind === 'MethodCall' && - macroTagsCalls.has(value.property.identifier.id)) - ); -} - -function isFbtJsxExpression( - fbtMacroTags: Set, - macroTagsCalls: Set, - value: InstructionValue, -): boolean { - return ( - value.kind === 'JsxExpression' && - ((value.tag.kind === 'Identifier' && - macroTagsCalls.has(value.tag.identifier.id)) || - (value.tag.kind === 'BuiltinTag' && - matchesExactTag(value.tag.name, fbtMacroTags))) - ); -} - -function isFbtJsxChild( - macroTagsCalls: Set, - lvalue: Place | null, - value: InstructionValue, -): boolean { - return ( - (value.kind === 'JsxExpression' || value.kind === 'JsxFragment') && - lvalue !== null && - macroTagsCalls.has(lvalue.identifier.id) - ); + return macroValues; } function expandFbtScopeRange( @@ -298,3 +283,22 @@ function expandFbtScopeRange( ); } } + +function visitOperands( + macroDefinition: MacroDefinition, + scope: ReactiveScope, + lvalue: Place, + value: InstructionValue, + macroValues: Set, + macroTags: Map, +): void { + macroValues.add(lvalue.identifier.id); + for (const operand of eachInstructionValueOperand(value)) { + if (macroDefinition.level === InlineLevel.Transitive) { + operand.identifier.scope = scope; + expandFbtScopeRange(scope.range, operand.identifier.mutableRange); + macroTags.set(operand.identifier.id, macroDefinition); + } + macroValues.add(operand.identifier.id); + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts index e84c1e57aae60..a574ecc16525e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Utils/TestUtils.ts @@ -135,16 +135,7 @@ function parseConfigPragmaEnvironmentForTest( } else if (val) { const parsedVal = tryParseTestPragmaValue(val).unwrap(); if (key === 'customMacros' && typeof parsedVal === 'string') { - const valSplit = parsedVal.split('.'); - const props = []; - for (const elt of valSplit.slice(1)) { - if (elt === '*') { - props.push({type: 'wildcard'}); - } else if (elt.length > 0) { - props.push({type: 'name', name: elt}); - } - } - maybeConfig[key] = [[valSplit[0], props]]; + maybeConfig[key] = [parsedVal.split('.')[0]]; continue; } maybeConfig[key] = parsedVal; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md index ac9bc2ce28a9a..96cea12a6231d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-leading-whitespace.expect.md @@ -44,15 +44,23 @@ import fbt from "fbt"; import { identity } from "shared-runtime"; function Component(props) { - const $ = _c(3); + const $ = _c(5); let t0; if ($[0] !== props.count || $[1] !== props.option) { + let t1; + if ($[3] !== props.count) { + t1 = identity(props.count); + $[3] = props.count; + $[4] = t1; + } else { + t1 = $[4]; + } t0 = ( {fbt._( { "*": "{count} votes for {option}", _1: "1 vote for {option}" }, [ - fbt._plural(identity(props.count), "count"), + fbt._plural(t1, "count"), fbt._param( "option", diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md index 9b4c29607d9e3..e5f465df169eb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-trailing-whitespace.expect.md @@ -44,15 +44,23 @@ import fbt from "fbt"; import { identity } from "shared-runtime"; function Component(props) { - const $ = _c(3); + const $ = _c(5); let t0; if ($[0] !== props.count || $[1] !== props.option) { + let t1; + if ($[3] !== props.count) { + t1 = identity(props.count); + $[3] = props.count; + $[4] = t1; + } else { + t1 = $[4]; + } t0 = ( {fbt._( { "*": "{count} votes for {option}", _1: "1 vote for {option}" }, [ - fbt._plural(identity(props.count), "count"), + fbt._plural(t1, "count"), fbt._param( "option", diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-two-subtrees.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-two-subtrees.expect.md index c1a1a5891b368..c421ccbe81021 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-two-subtrees.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-two-subtrees.expect.md @@ -37,7 +37,7 @@ import { c as _c } from "react/compiler-runtime"; import fbt from "fbt"; function Foo(t0) { - const $ = _c(7); + const $ = _c(13); const { name1, name2 } = t0; let t1; if ($[0] !== name1 || $[1] !== name2) { @@ -50,19 +50,34 @@ function Foo(t0) { t2 = $[4]; } let t3; - if ($[5] !== name2) { - t3 = {name2}; - $[5] = name2; - $[6] = t3; + if ($[5] !== name1 || $[6] !== t2) { + t3 = {t2}; + $[5] = name1; + $[6] = t2; + $[7] = t3; } else { - t3 = $[6]; + t3 = $[7]; + } + let t4; + if ($[8] !== name2) { + t4 = {name2}; + $[8] = name2; + $[9] = t4; + } else { + t4 = $[9]; + } + let t5; + if ($[10] !== name2 || $[11] !== t4) { + t5 = {t4}; + $[10] = name2; + $[11] = t4; + $[12] = t5; + } else { + t5 = $[12]; } t1 = fbt._( "{user1} and {user2} accepted your PR!", - [ - fbt._param("user1", {t2}), - fbt._param("user2", {t3}), - ], + [fbt._param("user1", t3), fbt._param("user2", t5)], { hk: "2PxMie" }, ); $[0] = name1; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md index 56ffb70cb0aeb..66141f7f8d788 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md @@ -29,20 +29,24 @@ import { c as _c } from "react/compiler-runtime"; import fbt from "fbt"; function Component(t0) { - const $ = _c(4); + const $ = _c(6); const { name, data, icon } = t0; let t1; if ($[0] !== data || $[1] !== icon || $[2] !== name) { + let t2; + if ($[4] !== name) { + t2 = {name}; + $[4] = name; + $[5] = t2; + } else { + t2 = $[5]; + } t1 = ( {fbt._( "{item author}{icon}{=m2}", [ - fbt._param( - "item author", - - {name}, - ), + fbt._param("item author", t2), fbt._param( "icon", diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md index 0ed51660b985e..c9a43bb075e88 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-fragment-value.expect.md @@ -27,16 +27,21 @@ import fbt from "fbt"; import { identity } from "shared-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(4); let t0; if ($[0] !== props.text) { + const t1 = identity(props.text); + let t2; + if ($[2] !== t1) { + t2 = <>{t1}; + $[2] = t1; + $[3] = t2; + } else { + t2 = $[3]; + } t0 = ( {identity(props.text)})], - { hk: "10F5Cc" }, - )} + value={fbt._("{value}%", [fbt._param("value", t2)], { hk: "10F5Cc" })} /> ); $[0] = props.text; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.expect.md new file mode 100644 index 0000000000000..93a9b869f670d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.expect.md @@ -0,0 +1,109 @@ + +## Input + +```javascript +// @flow +import {fbt} from 'fbt'; + +function Example({x}) { + // "Inner Text" needs to be visible to fbt: the element cannot + // be memoized separately + return ( + + Outer Text + + Inner Text + + + ); +} + +function Foo({x, children}) { + 'use no memo'; + return ( + <> +
{x}
+ {children} + + ); +} + +function Bar({children}) { + 'use no memo'; + return children; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Example, + params: [{x: 'Hello'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { fbt } from "fbt"; + +function Example(t0) { + const $ = _c(2); + const { x } = t0; + let t1; + if ($[0] !== x) { + t1 = fbt._( + "Outer Text {=m1}", + [ + fbt._implicitParam( + "=m1", + + + {fbt._( + "{=m1}", + [ + fbt._implicitParam( + "=m1", + + {fbt._("Inner Text", null, { hk: "32YB0l" })} + , + ), + ], + { hk: "23dJsI" }, + )} + , + ), + ], + { hk: "2RVA7V" }, + ); + $[0] = x; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +function Foo({ x, children }) { + "use no memo"; + return ( + <> +
{x}
+ {children} + + ); +} + +function Bar({ children }) { + "use no memo"; + return children; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Example, + params: [{ x: "Hello" }], +}; + +``` + +### Eval output +(kind: ok) Outer Text
Hello
Inner Text \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.js new file mode 100644 index 0000000000000..f89289fb039f3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.js @@ -0,0 +1,35 @@ +// @flow +import {fbt} from 'fbt'; + +function Example({x}) { + // "Inner Text" needs to be visible to fbt: the element cannot + // be memoized separately + return ( + + Outer Text + + Inner Text + + + ); +} + +function Foo({x, children}) { + 'use no memo'; + return ( + <> +
{x}
+ {children} + + ); +} + +function Bar({children}) { + 'use no memo'; + return children; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Example, + params: [{x: 'Hello'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.expect.md new file mode 100644 index 0000000000000..10a2a9ae131e9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.expect.md @@ -0,0 +1,128 @@ + +## Input + +```javascript +import fbt from 'fbt'; +import {Stringify, identity} from 'shared-runtime'; + +/** + * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. + * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error: + * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier + * --- + * t3 + * --- + */ +function Component({firstname, lastname}) { + 'use memo'; + return ( +
+ {fbt( + [ + 'Name: ', + fbt.param('firstname', ), + ', ', + fbt.param( + 'lastname', + identity( + fbt( + '(inner)' + + fbt.param('lastname', ), + 'Inner fbt value' + ) + ) + ), + ], + 'Name' + )} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{firstname: 'first', lastname: 'last'}], + sequentialRenders: [{firstname: 'first', lastname: 'last'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import fbt from "fbt"; +import { Stringify, identity } from "shared-runtime"; + +/** + * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. + * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error: + * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier + * --- + * t3 + * --- + */ +function Component(t0) { + "use memo"; + const $ = _c(9); + const { firstname, lastname } = t0; + let t1; + if ($[0] !== firstname || $[1] !== lastname) { + let t2; + if ($[3] !== firstname) { + t2 = ; + $[3] = firstname; + $[4] = t2; + } else { + t2 = $[4]; + } + let t3; + if ($[5] !== lastname) { + t3 = ; + $[5] = lastname; + $[6] = t3; + } else { + t3 = $[6]; + } + t1 = fbt._( + "Name: {firstname}, {lastname}", + [ + fbt._param("firstname", t2), + fbt._param( + "lastname", + identity( + fbt._("(inner){lastname}", [fbt._param("lastname", t3)], { + hk: "1Kdxyo", + }), + ), + ), + ], + { hk: "3AiIf8" }, + ); + $[0] = firstname; + $[1] = lastname; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[7] !== t1) { + t2 =
{t1}
; + $[7] = t1; + $[8] = t2; + } else { + t2 = $[8]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ firstname: "first", lastname: "last" }], + sequentialRenders: [{ firstname: "first", lastname: "last" }], +}; + +``` + +### Eval output +(kind: ok)
Name:
{"name":"first"}
, (inner)
{"name":"last"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.js new file mode 100644 index 0000000000000..07efb4e03a842 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt-jsx.js @@ -0,0 +1,42 @@ +import fbt from 'fbt'; +import {Stringify, identity} from 'shared-runtime'; + +/** + * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. + * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error: + * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier + * --- + * t3 + * --- + */ +function Component({firstname, lastname}) { + 'use memo'; + return ( +
+ {fbt( + [ + 'Name: ', + fbt.param('firstname', ), + ', ', + fbt.param( + 'lastname', + identity( + fbt( + '(inner)' + + fbt.param('lastname', ), + 'Inner fbt value' + ) + ) + ), + ], + 'Name' + )} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{firstname: 'first', lastname: 'last'}], + sequentialRenders: [{firstname: 'first', lastname: 'last'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.expect.md index 5ad089e1341bc..063e45e04e352 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.expect.md @@ -3,7 +3,7 @@ ```javascript import fbt from 'fbt'; -import {Stringify} from 'shared-runtime'; +import {identity} from 'shared-runtime'; /** * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. @@ -16,22 +16,25 @@ import {Stringify} from 'shared-runtime'; function Component({firstname, lastname}) { 'use memo'; return ( - +
{fbt( [ 'Name: ', - fbt.param('firstname', ), + fbt.param('firstname', identity(firstname)), ', ', fbt.param( 'lastname', - - {fbt('(inner fbt)', 'Inner fbt value')} - + identity( + fbt( + '(inner)' + fbt.param('lastname', identity(lastname)), + 'Inner fbt value' + ) + ) ), ], 'Name' )} - +
); } @@ -48,7 +51,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; import fbt from "fbt"; -import { Stringify } from "shared-runtime"; +import { identity } from "shared-runtime"; /** * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. @@ -70,14 +73,24 @@ function Component(t0) { fbt._param( "firstname", - , + identity(firstname), ), fbt._param( "lastname", - - {fbt._("(inner fbt)", null, { hk: "36qNwF" })} - , + identity( + fbt._( + "(inner){lastname}", + [ + fbt._param( + "lastname", + + identity(lastname), + ), + ], + { hk: "1Kdxyo" }, + ), + ), ), ], { hk: "3AiIf8" }, @@ -90,7 +103,7 @@ function Component(t0) { } let t2; if ($[3] !== t1) { - t2 = {t1}; + t2 =
{t1}
; $[3] = t1; $[4] = t2; } else { @@ -108,4 +121,4 @@ export const FIXTURE_ENTRYPOINT = { ``` ### Eval output -(kind: ok)
{"children":"Name: , "}
\ No newline at end of file +(kind: ok)
Name: first, (inner)last
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.js index 14e3278e395de..38465a628045b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.js @@ -1,5 +1,5 @@ import fbt from 'fbt'; -import {Stringify} from 'shared-runtime'; +import {identity} from 'shared-runtime'; /** * MemoizeFbtAndMacroOperands needs to account for nested fbt calls. @@ -12,22 +12,25 @@ import {Stringify} from 'shared-runtime'; function Component({firstname, lastname}) { 'use memo'; return ( - +
{fbt( [ 'Name: ', - fbt.param('firstname', ), + fbt.param('firstname', identity(firstname)), ', ', fbt.param( 'lastname', - - {fbt('(inner fbt)', 'Inner fbt value')} - + identity( + fbt( + '(inner)' + fbt.param('lastname', identity(lastname)), + 'Inner fbt value' + ) + ) ), ], 'Name' )} - +
); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md index 455c416d840e8..e004ef246abf6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md @@ -37,7 +37,7 @@ function Component(props) { const $ = _c(16); let t0; if ($[0] !== props) { - t0 = idx(props, _temp); + t0 = idx(props, (_) => _.group.label); $[0] = props; $[1] = t0; } else { @@ -46,7 +46,7 @@ function Component(props) { const groupName1 = t0; let t1; if ($[2] !== props) { - t1 = idx.a(props, _temp2); + t1 = idx.a(props, (__0) => __0.group.label); $[2] = props; $[3] = t1; } else { @@ -108,12 +108,6 @@ function Component(props) { } return t5; } -function _temp2(__0) { - return __0.group.label; -} -function _temp(_) { - return _.group.label; -} ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md index cc5a4200a94be..e98fb191ce73a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md @@ -31,7 +31,7 @@ function Component(props) { const $ = _c(10); let t0; if ($[0] !== props) { - t0 = idx(props, _temp); + t0 = idx(props, (_) => _.group.label); $[0] = props; $[1] = t0; } else { @@ -74,9 +74,6 @@ function Component(props) { } return t3; } -function _temp(_) { - return _.group.label; -} ``` \ No newline at end of file