From 46fa6c2effe3fac5c3e47f287c04baed260687f1 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 16 Oct 2025 21:42:46 -0700 Subject: [PATCH] [compiler] More fbt compatibility 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. --- .../src/Entrypoint/Pipeline.ts | 2 +- .../src/HIR/Environment.ts | 12 +- .../MemoizeFbtAndMacroOperandsInSameScope.ts | 434 +++++++++--------- .../ReactiveScopes/PromoteUsedTemporaries.ts | 10 +- .../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 ++ .../fbt/repro-fbt-param-nested-fbt.expect.md | 52 ++- ...idx-method-no-outlining-wildcard.expect.md | 10 +- .../idx-method-no-outlining.expect.md | 5 +- 15 files changed, 476 insertions(+), 296 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 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 1085d4c69e..035ca920ba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -499,7 +499,7 @@ function runWithEnvironment( value: reactiveFunction, }); - promoteUsedTemporaries(reactiveFunction); + promoteUsedTemporaries(reactiveFunction, fbtOperands); log({ kind: 'reactive', name: 'PromoteUsedTemporaries', 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 4ec4a4a795..bdbfb20a59 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 4ae9978b58..0ce05a823e 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>>(); + const macroValues = mergeMacroArguments(fn, macroTags, macroKinds); - 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); - } - - 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); - } - } 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 'LoadGlobal': { + let macroDefinition = macroKinds.get(value.binding.name); + if (macroDefinition != null) { + macroTags.set(lvalue.identifier.id, macroDefinition); } + break; + } + 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/ReactiveScopes/PromoteUsedTemporaries.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts index ce3ac13a9f..3958e9fb5c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts @@ -177,6 +177,7 @@ type State = { DeclarationId, {activeScopes: Array; usedOutsideScope: boolean} >; // true if referenced within another scope, false if only accessed outside of scopes + fbtOperands: Set; }; /** @@ -270,7 +271,8 @@ class PromoteInterposedTemporaries extends ReactiveFunctionVisitor { if ( needsPromotion && identifier.name === null && - !this.#consts.has(identifier.id) + !this.#consts.has(identifier.id) && + !this.#promotable.fbtOperands.has(place.identifier.id) ) { /* * If the identifier hasn't been promoted but is marked as needing @@ -429,11 +431,15 @@ class PromoteInterposedTemporaries extends ReactiveFunctionVisitor { } } -export function promoteUsedTemporaries(fn: ReactiveFunction): void { +export function promoteUsedTemporaries( + fn: ReactiveFunction, + fbtOperands: Set, +): void { const state: State = { tags: new Set(), promoted: new Set(), pruned: new Map(), + fbtOperands, }; visitReactiveFunction(fn, new CollectPromotableTemporaries(), state); for (const operand of fn.params) { 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 e84c1e57aa..a574ecc165 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 ac9bc2ce28..96cea12a62 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 9b4c29607d..e5f465df16 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 c1a1a5891b..c421ccbe81 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 56ffb70cb0..66141f7f8d 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 0ed51660b9..c9a43bb075 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 0000000000..93a9b869f6 --- /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 0000000000..f89289fb03 --- /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.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.expect.md index 5ad089e134..3aac8a9053 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 @@ -60,26 +60,40 @@ import { Stringify } from "shared-runtime"; */ function Component(t0) { "use memo"; - const $ = _c(5); + const $ = _c(10); 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] === Symbol.for("react.memo_cache_sentinel")) { + t3 = fbt._("(inner fbt)", null, { hk: "36qNwF" }); + $[5] = t3; + } else { + t3 = $[5]; + } + let t4; + if ($[6] !== lastname) { + t4 = ( + + {t3} + + ); + $[6] = lastname; + $[7] = t4; + } else { + t4 = $[7]; + } t1 = fbt._( "Name: {firstname}, {lastname}", - [ - fbt._param( - "firstname", - - , - ), - fbt._param( - "lastname", - - - {fbt._("(inner fbt)", null, { hk: "36qNwF" })} - , - ), - ], + [fbt._param("firstname", t2), fbt._param("lastname", t4)], { hk: "3AiIf8" }, ); $[0] = firstname; @@ -89,12 +103,12 @@ function Component(t0) { t1 = $[2]; } let t2; - if ($[3] !== t1) { + if ($[8] !== t1) { t2 = {t1}; - $[3] = t1; - $[4] = t2; + $[8] = t1; + $[9] = t2; } else { - t2 = $[4]; + t2 = $[9]; } return t2; } 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 455c416d84..e004ef246a 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 cc5a4200a9..e98fb191ce 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