@samitouri / QOS-React / commits / 85f415e33b

[compiler] Fix fbt for the ∞th time (#34865)

We now do a single pass over the HIR, building up two data structures: * One tracks values that are known macro tags or macro calls. * One tracks operands of macro-related instructions so that we can later group them. After building up these data structures, we do a pass over the latter structure. For each macro call instruction, we recursively traverse its operands to ensure they're in the same scope. Thus, something like `fbt('hello' + fbt.param(foo(), "..."))` will correctly merge the fbt call, the `+` binary expression, the `fbt.param()` call, and `foo()` into a single scope. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34865). * #34855 * __->__ #34865

Joseph Savona committed Oct 15, 2025 at 16:23 UTC 85f415e33b95d65aaa29f92268b31d33060628ac
9 files changed +341 -165
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts
+109 -86
@@ -7,14 +7,17 @@
7
8 import {
9 HIRFunction,
10 + Identifier,
11 IdentifierId,
12 + InstructionValue,
13 makeInstructionId,
14 MutableRange,
15 Place,
14 - ReactiveValue,
16 + ReactiveScope,
17 } from '../HIR';
18 import {Macro, MacroMethod} from '../HIR/Environment';
17 -import {eachReactiveValueOperand} from './visitors';
19 +import {eachInstructionValueOperand} from '../HIR/visitors';
20 +import {Iterable_some} from '../Utils/utils';
21
22 /**
23 * This pass supports the `fbt` translation system (https://facebook.github.io/fbt/)
@@ -48,24 +51,49 @@ export function memoizeFbtAndMacroOperandsInSameScope(
51 ...Array.from(FBT_TAGS).map((tag): Macro => [tag, []]),
52 ...(fn.env.config.customMacros ?? []),
53 ]);
51 - const fbtValues: Set<IdentifierId> = new Set();
54 + /**
55 + * Set of all identifiers that load fbt or other macro functions or their nested
56 + * properties, as well as values known to be the results of invoking macros
57 + */
58 + const macroTagsCalls: Set<IdentifierId> = new Set();
59 + /**
60 + * Mapping of lvalue => list of operands for all expressions where either
61 + * the lvalue is a known fbt/macro call and/or the operands transitively
62 + * contain fbt/macro calls.
63 + *
64 + * This is the key data structure that powers the scope merging: we start
65 + * at the lvalues and merge operands into the lvalue's scope.
66 + */
67 + const macroValues: Map<Identifier, Array<Identifier>> = new Map();
68 + // Tracks methods loaded from macros, like fbt.param or idx.foo
69 const macroMethods = new Map<IdentifierId, Array<Array<MacroMethod>>>();
53 - while (true) {
54 - let vsize = fbtValues.size;
55 - let msize = macroMethods.size;
56 - visit(fn, fbtMacroTags, fbtValues, macroMethods);
57 - if (vsize === fbtValues.size && msize === macroMethods.size) {
58 - break;
70 +
71 + visit(fn, fbtMacroTags, macroTagsCalls, macroMethods, macroValues);
72 +
73 + for (const root of macroValues.keys()) {
74 + const scope = root.scope;
75 + if (scope == null) {
76 + continue;
77 + }
78 + // Merge the operands into the same scope if this is a known macro invocation
79 + if (!macroTagsCalls.has(root.id)) {
80 + continue;
81 }
82 + mergeScopes(root, scope, macroValues, macroTagsCalls);
83 }
61 - return fbtValues;
84 +
85 + return macroTagsCalls;
86 }
87
88 export const FBT_TAGS: Set<string> = new Set([
89 'fbt',
90 'fbt:param',
91 + 'fbt:enum',
92 + 'fbt:plural',
93 'fbs',
94 'fbs:param',
95 + 'fbs:enum',
96 + 'fbs:plural',
97 ]);
98 export const SINGLE_CHILD_FBT_TAGS: Set<string> = new Set([
99 'fbt:param',
@@ -75,10 +103,22 @@ export const SINGLE_CHILD_FBT_TAGS: Set<string> = new Set([
103 function visit(
104 fn: HIRFunction,
105 fbtMacroTags: Set<Macro>,
78 - fbtValues: Set<IdentifierId>,
106 + macroTagsCalls: Set<IdentifierId>,
107 macroMethods: Map<IdentifierId, Array<Array<MacroMethod>>>,
108 + macroValues: Map<Identifier, Array<Identifier>>,
109 ): void {
110 for (const [, block] of fn.body.blocks) {
111 + for (const phi of block.phis) {
112 + const macroOperands: Array<Identifier> = [];
113 + for (const operand of phi.operands.values()) {
114 + if (macroValues.has(operand.identifier)) {
115 + macroOperands.push(operand.identifier);
116 + }
117 + }
118 + if (macroOperands.length !== 0) {
119 + macroValues.set(phi.place.identifier, macroOperands);
120 + }
121 + }
122 for (const instruction of block.instructions) {
123 const {lvalue, value} = instruction;
124 if (lvalue === null) {
@@ -93,13 +133,13 @@ function visit(
133 * We don't distinguish between tag names and strings, so record
134 * all `fbt` string literals in case they are used as a jsx tag.
135 */
96 - fbtValues.add(lvalue.identifier.id);
136 + macroTagsCalls.add(lvalue.identifier.id);
137 } else if (
138 value.kind === 'LoadGlobal' &&
139 matchesExactTag(value.binding.name, fbtMacroTags)
140 ) {
141 // Record references to `fbt` as a global
102 - fbtValues.add(lvalue.identifier.id);
142 + macroTagsCalls.add(lvalue.identifier.id);
143 } else if (
144 value.kind === 'LoadGlobal' &&
145 matchTagRoot(value.binding.name, fbtMacroTags) !== null
@@ -121,84 +161,66 @@ function visit(
161 if (method.length > 1) {
162 newMethods.push(method.slice(1));
163 } else {
124 - fbtValues.add(lvalue.identifier.id);
164 + macroTagsCalls.add(lvalue.identifier.id);
165 }
166 }
167 }
168 if (newMethods.length > 0) {
169 macroMethods.set(lvalue.identifier.id, newMethods);
170 }
131 - } else if (isFbtCallExpression(fbtValues, value)) {
132 - const fbtScope = lvalue.identifier.scope;
133 - if (fbtScope === null) {
134 - continue;
135 - }
136 -
137 - /*
138 - * if the JSX element's tag was `fbt`, mark all its operands
139 - * to ensure that they end up in the same scope as the jsx element
140 - * itself.
141 - */
142 - for (const operand of eachReactiveValueOperand(value)) {
143 - operand.identifier.scope = fbtScope;
144 -
145 - // Expand the jsx element's range to account for its operands
146 - expandFbtScopeRange(fbtScope.range, operand.identifier.mutableRange);
147 - fbtValues.add(operand.identifier.id);
148 - }
171 } else if (
150 - isFbtJsxExpression(fbtMacroTags, fbtValues, value) ||
151 - isFbtJsxChild(fbtValues, lvalue, value)
172 + value.kind === 'PropertyLoad' &&
173 + macroTagsCalls.has(value.object.identifier.id)
174 ) {
153 - const fbtScope = lvalue.identifier.scope;
154 - if (fbtScope === null) {
155 - continue;
156 - }
157 -
158 - /*
159 - * if the JSX element's tag was `fbt`, mark all its operands
160 - * to ensure that they end up in the same scope as the jsx element
161 - * itself.
162 - */
163 - for (const operand of eachReactiveValueOperand(value)) {
164 - operand.identifier.scope = fbtScope;
165 -
166 - // Expand the jsx element's range to account for its operands
167 - expandFbtScopeRange(fbtScope.range, operand.identifier.mutableRange);
168 -
169 - /*
170 - * NOTE: we add the operands as fbt values so that they are also
171 - * grouped with this expression
172 - */
173 - fbtValues.add(operand.identifier.id);
174 - }
175 - } else if (fbtValues.has(lvalue.identifier.id)) {
176 - const fbtScope = lvalue.identifier.scope;
177 - if (fbtScope === null) {
178 - return;
179 - }
180 -
181 - for (const operand of eachReactiveValueOperand(value)) {
182 - if (
183 - operand.identifier.name !== null &&
184 - operand.identifier.name.kind === 'named'
185 - ) {
186 - /*
187 - * named identifiers were already locals, we only have to force temporaries
188 - * into the same scope
189 - */
190 - continue;
175 + macroTagsCalls.add(lvalue.identifier.id);
176 + } else if (
177 + isFbtJsxExpression(fbtMacroTags, macroTagsCalls, value) ||
178 + isFbtJsxChild(macroTagsCalls, lvalue, value) ||
179 + isFbtCallExpression(macroTagsCalls, value)
180 + ) {
181 + macroTagsCalls.add(lvalue.identifier.id);
182 + macroValues.set(
183 + lvalue.identifier,
184 + Array.from(
185 + eachInstructionValueOperand(value),
186 + operand => operand.identifier,
187 + ),
188 + );
189 + } else if (
190 + Iterable_some(eachInstructionValueOperand(value), operand =>
191 + macroValues.has(operand.identifier),
192 + )
193 + ) {
194 + const macroOperands: Array<Identifier> = [];
195 + for (const operand of eachInstructionValueOperand(value)) {
196 + if (macroValues.has(operand.identifier)) {
197 + macroOperands.push(operand.identifier);
198 }
192 - operand.identifier.scope = fbtScope;
193 -
194 - // Expand the jsx element's range to account for its operands
195 - expandFbtScopeRange(fbtScope.range, operand.identifier.mutableRange);
199 }
200 + macroValues.set(lvalue.identifier, macroOperands);
201 }
202 }
203 }
204 }
205
206 +function mergeScopes(
207 + root: Identifier,
208 + scope: ReactiveScope,
209 + macroValues: Map<Identifier, Array<Identifier>>,
210 + macroTagsCalls: Set<IdentifierId>,
211 +): void {
212 + const operands = macroValues.get(root);
213 + if (operands == null) {
214 + return;
215 + }
216 + for (const operand of operands) {
217 + operand.scope = scope;
218 + expandFbtScopeRange(scope.range, operand.mutableRange);
219 + macroTagsCalls.add(operand.id);
220 + mergeScopes(operand, scope, macroValues, macroTagsCalls);
221 + }
222 +}
223 +
224 function matchesExactTag(s: string, tags: Set<Macro>): boolean {
225 return Array.from(tags).some(macro =>
226 typeof macro === 'string'
@@ -229,39 +251,40 @@ function matchTagRoot(
251 }
252
253 function isFbtCallExpression(
232 - fbtValues: Set<IdentifierId>,
233 - value: ReactiveValue,
254 + macroTagsCalls: Set<IdentifierId>,
255 + value: InstructionValue,
256 ): boolean {
257 return (
258 (value.kind === 'CallExpression' &&
237 - fbtValues.has(value.callee.identifier.id)) ||
238 - (value.kind === 'MethodCall' && fbtValues.has(value.property.identifier.id))
259 + macroTagsCalls.has(value.callee.identifier.id)) ||
260 + (value.kind === 'MethodCall' &&
261 + macroTagsCalls.has(value.property.identifier.id))
262 );
263 }
264
265 function isFbtJsxExpression(
266 fbtMacroTags: Set<Macro>,
244 - fbtValues: Set<IdentifierId>,
245 - value: ReactiveValue,
267 + macroTagsCalls: Set<IdentifierId>,
268 + value: InstructionValue,
269 ): boolean {
270 return (
271 value.kind === 'JsxExpression' &&
272 ((value.tag.kind === 'Identifier' &&
250 - fbtValues.has(value.tag.identifier.id)) ||
273 + macroTagsCalls.has(value.tag.identifier.id)) ||
274 (value.tag.kind === 'BuiltinTag' &&
275 matchesExactTag(value.tag.name, fbtMacroTags)))
276 );
277 }
278
279 function isFbtJsxChild(
257 - fbtValues: Set<IdentifierId>,
280 + macroTagsCalls: Set<IdentifierId>,
281 lvalue: Place | null,
259 - value: ReactiveValue,
282 + value: InstructionValue,
283 ): boolean {
284 return (
285 (value.kind === 'JsxExpression' || value.kind === 'JsxFragment') &&
286 lvalue !== null &&
264 - fbtValues.has(lvalue.identifier.id)
287 + macroTagsCalls.has(lvalue.identifier.id)
288 );
289 }
290
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-nested-fbt.expect.md deleted
-56
@@ -1,56 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -import fbt from 'fbt';
6 -import {Stringify} from 'shared-runtime';
7 -
8 -/**
9 - * MemoizeFbtAndMacroOperands needs to account for nested fbt calls.
10 - * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error:
11 - * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier
12 - * ---
13 - * t3
14 - * ---
15 - */
16 -function Component({firstname, lastname}) {
17 - 'use memo';
18 - return (
19 - <Stringify>
20 - {fbt(
21 - [
22 - 'Name: ',
23 - fbt.param('firstname', <Stringify key={0} name={firstname} />),
24 - ', ',
25 - fbt.param(
26 - 'lastname',
27 - <Stringify key={0} name={lastname}>
28 - {fbt('(inner fbt)', 'Inner fbt value')}
29 - </Stringify>
30 - ),
31 - ],
32 - 'Name'
33 - )}
34 - </Stringify>
35 - );
36 -}
37 -
38 -export const FIXTURE_ENTRYPOINT = {
39 - fn: Component,
40 - params: [{firstname: 'first', lastname: 'last'}],
41 - sequentialRenders: [{firstname: 'first', lastname: 'last'}],
42 -};
43 -
44 -```
45 -
46 -
47 -## Error
48 -
49 -```
50 -Line 19 Column 11: fbt: unsupported babel node: Identifier
51 ----
52 -t3
53 ----
54 -```
55 -
56 -
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-two-subtrees.expect.md
+19 -15
@@ -37,27 +37,31 @@ import { c as _c } from "react/compiler-runtime";
37 import fbt from "fbt";
38
39 function Foo(t0) {
40 - const $ = _c(3);
40 + const $ = _c(7);
41 const { name1, name2 } = t0;
42 let t1;
43 if ($[0] !== name1 || $[1] !== name2) {
44 + let t2;
45 + if ($[3] !== name1) {
46 + t2 = <b>{name1}</b>;
47 + $[3] = name1;
48 + $[4] = t2;
49 + } else {
50 + t2 = $[4];
51 + }
52 + let t3;
53 + if ($[5] !== name2) {
54 + t3 = <b>{name2}</b>;
55 + $[5] = name2;
56 + $[6] = t3;
57 + } else {
58 + t3 = $[6];
59 + }
60 t1 = fbt._(
61 "{user1} and {user2} accepted your PR!",
62 [
47 - fbt._param(
48 - "user1",
49 -
50 - <span key={name1}>
51 - <b>{name1}</b>
52 - </span>,
53 - ),
54 - fbt._param(
55 - "user2",
56 -
57 - <span key={name2}>
58 - <b>{name2}</b>
59 - </span>,
60 - ),
63 + fbt._param("user1", <span key={name1}>{t2}</span>),
64 + fbt._param("user2", <span key={name2}>{t3}</span>),
65 ],
66 { hk: "2PxMie" },
67 );
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.expect.md new
+111
@@ -0,0 +1,111 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import fbt from 'fbt';
6 +import {Stringify} from 'shared-runtime';
7 +
8 +/**
9 + * MemoizeFbtAndMacroOperands needs to account for nested fbt calls.
10 + * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error:
11 + * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier
12 + * ---
13 + * t3
14 + * ---
15 + */
16 +function Component({firstname, lastname}) {
17 + 'use memo';
18 + return (
19 + <Stringify>
20 + {fbt(
21 + [
22 + 'Name: ',
23 + fbt.param('firstname', <Stringify key={0} name={firstname} />),
24 + ', ',
25 + fbt.param(
26 + 'lastname',
27 + <Stringify key={0} name={lastname}>
28 + {fbt('(inner fbt)', 'Inner fbt value')}
29 + </Stringify>
30 + ),
31 + ],
32 + 'Name'
33 + )}
34 + </Stringify>
35 + );
36 +}
37 +
38 +export const FIXTURE_ENTRYPOINT = {
39 + fn: Component,
40 + params: [{firstname: 'first', lastname: 'last'}],
41 + sequentialRenders: [{firstname: 'first', lastname: 'last'}],
42 +};
43 +
44 +```
45 +
46 +## Code
47 +
48 +```javascript
49 +import { c as _c } from "react/compiler-runtime";
50 +import fbt from "fbt";
51 +import { Stringify } from "shared-runtime";
52 +
53 +/**
54 + * MemoizeFbtAndMacroOperands needs to account for nested fbt calls.
55 + * Expected fixture `fbt-param-call-arguments` to succeed but it failed with error:
56 + * /fbt-param-call-arguments.ts: Line 19 Column 11: fbt: unsupported babel node: Identifier
57 + * ---
58 + * t3
59 + * ---
60 + */
61 +function Component(t0) {
62 + "use memo";
63 + const $ = _c(5);
64 + const { firstname, lastname } = t0;
65 + let t1;
66 + if ($[0] !== firstname || $[1] !== lastname) {
67 + t1 = fbt._(
68 + "Name: {firstname}, {lastname}",
69 + [
70 + fbt._param(
71 + "firstname",
72 +
73 + <Stringify key={0} name={firstname} />,
74 + ),
75 + fbt._param(
76 + "lastname",
77 +
78 + <Stringify key={0} name={lastname}>
79 + {fbt._("(inner fbt)", null, { hk: "36qNwF" })}
80 + </Stringify>,
81 + ),
82 + ],
83 + { hk: "3AiIf8" },
84 + );
85 + $[0] = firstname;
86 + $[1] = lastname;
87 + $[2] = t1;
88 + } else {
89 + t1 = $[2];
90 + }
91 + let t2;
92 + if ($[3] !== t1) {
93 + t2 = <Stringify>{t1}</Stringify>;
94 + $[3] = t1;
95 + $[4] = t2;
96 + } else {
97 + t2 = $[4];
98 + }
99 + return t2;
100 +}
101 +
102 +export const FIXTURE_ENTRYPOINT = {
103 + fn: Component,
104 + params: [{ firstname: "first", lastname: "last" }],
105 + sequentialRenders: [{ firstname: "first", lastname: "last" }],
106 +};
107 +
108 +```
109 +
110 +### Eval output
111 +(kind: ok) <div>{"children":"Name: , "}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-fbt-param-nested-fbt.js renamed
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-separately-memoized-fbt-param.expect.md new
+78
@@ -0,0 +1,78 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {fbt} from 'fbt';
6 +import {useState} from 'react';
7 +
8 +const MIN = 10;
9 +
10 +function Component() {
11 + const [count, setCount] = useState(0);
12 +
13 + return fbt(
14 + 'Expected at least ' +
15 + fbt.param('min', MIN, {number: true}) +
16 + ' items, but got ' +
17 + fbt.param('count', count, {number: true}) +
18 + ' items.',
19 + 'Error description'
20 + );
21 +}
22 +
23 +export const FIXTURE_ENTRYPOINT = {
24 + fn: Component,
25 + params: [{}],
26 +};
27 +
28 +```
29 +
30 +## Code
31 +
32 +```javascript
33 +import { c as _c } from "react/compiler-runtime";
34 +import { fbt } from "fbt";
35 +import { useState } from "react";
36 +
37 +const MIN = 10;
38 +
39 +function Component() {
40 + const $ = _c(2);
41 + const [count] = useState(0);
42 + let t0;
43 + if ($[0] !== count) {
44 + t0 = fbt._(
45 + { "*": { "*": "Expected at least {min} items, but got {count} items." } },
46 + [
47 + fbt._param(
48 + "min",
49 +
50 + MIN,
51 + [0],
52 + ),
53 + fbt._param(
54 + "count",
55 +
56 + count,
57 + [0],
58 + ),
59 + ],
60 + { hk: "36gbz8" },
61 + );
62 + $[0] = count;
63 + $[1] = t0;
64 + } else {
65 + t0 = $[1];
66 + }
67 + return t0;
68 +}
69 +
70 +export const FIXTURE_ENTRYPOINT = {
71 + fn: Component,
72 + params: [{}],
73 +};
74 +
75 +```
76 +
77 +### Eval output
78 +(kind: ok) Expected at least 10 items, but got 0 items.
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/repro-separately-memoized-fbt-param.js new
+22
@@ -0,0 +1,22 @@
1 +import {fbt} from 'fbt';
2 +import {useState} from 'react';
3 +
4 +const MIN = 10;
5 +
6 +function Component() {
7 + const [count, setCount] = useState(0);
8 +
9 + return fbt(
10 + 'Expected at least ' +
11 + fbt.param('min', MIN, {number: true}) +
12 + ' items, but got ' +
13 + fbt.param('count', count, {number: true}) +
14 + ' items.',
15 + 'Error description'
16 + );
17 +}
18 +
19 +export const FIXTURE_ENTRYPOINT = {
20 + fn: Component,
21 + params: [{}],
22 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md
+1 -4
@@ -73,7 +73,7 @@ function Component(props) {
73 const groupName4 = t3;
74 let t4;
75 if ($[8] !== props) {
76 - t4 = idx.hello_world.b.c(props, _temp3);
76 + t4 = idx.hello_world.b.c(props, (__3) => __3.group.label);
77 $[8] = props;
78 $[9] = t4;
79 } else {
@@ -108,9 +108,6 @@ function Component(props) {
108 }
109 return t5;
110 }
111 -function _temp3(__3) {
112 - return __3.group.label;
113 -}
111 function _temp2(__0) {
112 return __0.group.label;
113 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md
+1 -4
@@ -49,7 +49,7 @@ function Component(props) {
49 const groupName2 = t1;
50 let t2;
51 if ($[4] !== props) {
52 - t2 = idx.a.b(props, _temp2);
52 + t2 = idx.a.b(props, (__1) => __1.group.label);
53 $[4] = props;
54 $[5] = t2;
55 } else {
@@ -74,9 +74,6 @@ function Component(props) {
74 }
75 return t3;
76 }
77 -function _temp2(__1) {
78 - return __1.group.label;
79 -}
77 function _temp(_) {
78 return _.group.label;
79 }