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..7c28789232 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import {MakeDirectoryOptions} from 'fs'; import { HIRFunction, Identifier, @@ -15,9 +16,61 @@ 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'; +import {printPlace} from '../HIR/PrintHIR'; + +/** + * How deeply does the macro require its arguments to be inlined? + * Shallow means that only the top-level expressions must be inlined: + * + * ``` + * macro(foo(bar())) + * => shallow inline => + * macro(foo(t0)) + * ``` + * + * While transitive means that everything below must be inlined: + * + * ``` + * + * + * + * + * Text // inline all the way down to this + * + * + * + * + * ``` + * + * However, a shallow inlined macro acts as a stopping point for + * inlining of a parent transitive one. So for example we have + * to inline from "downward" but switch to shallow mode + * for any nested + */ +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]]), +}; /** * This pass supports the `fbt` translation system (https://facebook.github.io/fbt/) @@ -47,245 +100,208 @@ import {Iterable_some} from '../Utils/utils'; 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', SHALLOW_MACRO], + ['fbt:plural', SHALLOW_MACRO], + ['fbs', FBT_MACRO], + ['fbs:param', SHALLOW_MACRO], + ['fbs:enum', SHALLOW_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 '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(lvalue.identifier.id) ?? + macroTags.get(callee.identifier.id); + if (macroDefinition != null) { + macroValues.add(lvalue.identifier.id); + for (const operand of eachInstructionValueOperand(value)) { + macroValues.add(operand.identifier.id); + operand.identifier.scope = scope; + expandFbtScopeRange(scope.range, operand.identifier.mutableRange); + if (macroDefinition.level === InlineLevel.Transitive) { + macroTags.set(operand.identifier.id, macroDefinition); + } + } + } + break; + } + case 'JsxExpression': { + const scope = lvalue.identifier.scope; + if (scope == null) { + continue; + } + let macroDefinition = macroTags.get(lvalue.identifier.id); + if (macroDefinition == null) { + if (value.tag.kind === 'Identifier') { + macroDefinition = macroTags.get(value.tag.identifier.id); + } else { + macroDefinition = macroKinds.get(value.tag.name); + } + } + if (macroDefinition != null) { + macroValues.add(lvalue.identifier.id); + for (const operand of eachInstructionValueOperand(value)) { + macroValues.add(operand.identifier.id); + operand.identifier.scope = scope; + expandFbtScopeRange(scope.range, operand.identifier.mutableRange); + if (macroDefinition.level === InlineLevel.Transitive) { + macroTags.set(operand.identifier.id, macroDefinition); + } + } + } + break; + } + // case 'JSXText': + // case 'Primitive': + // case 'TemplateLiteral': + 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; + } + default: { + const scope = lvalue.identifier.scope; + if (scope == null) { + continue; + } + const macroDefinition = macroTags.get(lvalue.identifier.id); + if (macroDefinition != null) { + macroValues.add(lvalue.identifier.id); + for (const operand of eachInstructionValueOperand(value)) { + macroValues.add(operand.identifier.id); + operand.identifier.scope = scope; + expandFbtScopeRange(scope.range, operand.identifier.mutableRange); + if (macroDefinition.level === InlineLevel.Transitive) { + macroTags.set(operand.identifier.id, macroDefinition); + } + } + } + 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) { + continue; + } + macroValues.add(phi.place.identifier.id); + for (const operand of phi.operands.values()) { + macroValues.add(operand.identifier.id); + operand.identifier.scope = scope; + expandFbtScopeRange(scope.range, operand.identifier.mutableRange); + macroTags.set(operand.identifier.id, macroDefinition); + } } } - 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( 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-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..1acfd65d16 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,31 +37,27 @@ import { c as _c } from "react/compiler-runtime"; import fbt from "fbt"; function Foo(t0) { - const $ = _c(7); + const $ = _c(3); const { name1, name2 } = t0; let t1; if ($[0] !== name1 || $[1] !== name2) { - let t2; - if ($[3] !== name1) { - t2 = {name1}; - $[3] = name1; - $[4] = t2; - } else { - t2 = $[4]; - } - let t3; - if ($[5] !== name2) { - t3 = {name2}; - $[5] = name2; - $[6] = t3; - } else { - t3 = $[6]; - } t1 = fbt._( "{user1} and {user2} accepted your PR!", [ - fbt._param("user1", {t2}), - fbt._param("user2", {t3}), + fbt._param( + "user1", + + + {name1} + , + ), + fbt._param( + "user2", + + + {name2} + , + ), ], { hk: "2PxMie" }, ); 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..df13a910da --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.expect.md @@ -0,0 +1,67 @@ + +## 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 + + + ); +} + +``` + +## 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; +} + +``` + +### 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/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..2021af08f6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/recursively-merge-scopes-jsx.js @@ -0,0 +1,15 @@ +// @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 + + + ); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-macro-property-not-handled.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-macro-property-not-handled.expect.md index a06b283d04..780deffac3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-macro-property-not-handled.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-macro-property-not-handled.expect.md @@ -31,7 +31,6 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; import fbt from "fbt"; import { useIdentity } from "shared-runtime"; @@ -41,23 +40,14 @@ import { useIdentity } from "shared-runtime"; * `importSpecifier.funcName` (see https://fburl.com/code/72icxwmn) */ function useFoo(t0) { - const $ = _c(2); const { items } = t0; - let t1; - if ($[0] !== items) { - t1 = [...items]; - $[0] = items; - $[1] = t1; - } else { - t1 = $[1]; - } return fbt._( { "*": "There are {number of items} items", _1: "There is {number of items} items", }, [ - fbt._plural(useIdentity(t1).length), + fbt._plural(useIdentity([...items]).length), fbt._param( "number of items", 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