Memoize fbt children in same scope
Fixes another special-case rule of fbt that i wasn't aware of: apparently `<fbt:param>` elements don't have to appear as direct children of `<fbt>`, they can be nested, and in this case they must appear as direct children of the fbt and not via an identifier indirection. This PR recursively extends the scope of FBT operands to make this work.
Joe Savona committed
Sep 15, 2023 at 10:31 UTC
e5e71dfd59ab8629235dd5ad5bd5164c171471c1
7 files changed
+208
-25
compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts
+6
-1
@@ -55,7 +55,11 @@ import {
55
} from "../ReactiveScopes";
56
import { eliminateRedundantPhi, enterSSA, leaveSSA } from "../SSA";
57
import { inferTypes } from "../TypeInference";
58
-import { logHIRFunction, logReactiveFunction } from "../Utils/logger";
58
+import {
59
+ logCodegenFunction,
60
+ logHIRFunction,
61
+ logReactiveFunction,
62
+} from "../Utils/logger";
63
import { assertExhaustive } from "../Utils/utils";
64
import {
65
validateFrozenLambdas,
@@ -319,6 +323,7 @@ export function compileFn(
323
export function log(value: CompilerPipelineValue): CompilerPipelineValue {
324
switch (value.kind) {
325
case "ast": {
326
+ logCodegenFunction(value.name, value.value);
327
break;
328
}
329
case "hir": {
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts
+57
-6
@@ -8,6 +8,7 @@
8
import {
9
IdentifierId,
10
makeInstructionId,
11
+ Place,
12
ReactiveFunction,
13
ReactiveInstruction,
14
ReactiveValue,
@@ -33,9 +34,18 @@ import {
34
* to be independently memoized
35
*/
36
export function memoizeFbtOperandsInSameScope(fn: ReactiveFunction): void {
36
- visitReactiveFunction(fn, new Transform(), undefined);
37
+ const transform = new Transform();
38
+ while (true) {
39
+ let size = transform.fbtValues.size;
40
+ visitReactiveFunction(fn, transform, undefined);
41
+ if (size === transform.fbtValues.size) {
42
+ break;
43
+ }
44
+ }
45
}
46
47
+const FBT_TAGS: Set<string> = new Set(["fbt", "fbt:param"]);
48
+
49
class Transform extends ReactiveFunctionVisitor<void> {
50
// Values that represent *potential* references of `fbt` as a JSX tag name
51
// or as a callee.
@@ -52,18 +62,34 @@ class Transform extends ReactiveFunctionVisitor<void> {
62
if (
63
value.kind === "Primitive" &&
64
typeof value.value === "string" &&
55
- value.value === "fbt"
65
+ FBT_TAGS.has(value.value)
66
) {
67
// We don't distinguish between tag names and strings, so record
68
// all `fbt` string literals in case they are used as a jsx tag.
69
this.fbtValues.add(lvalue.identifier.id);
60
- } else if (value.kind === "LoadGlobal" && value.name === "fbt") {
70
+ } else if (value.kind === "LoadGlobal" && FBT_TAGS.has(value.name)) {
71
// Record references to `fbt` as a global
72
this.fbtValues.add(lvalue.identifier.id);
73
+ } else if (isFbtCallExpression(this.fbtValues, value)) {
74
+ const fbtScope = lvalue.identifier.scope;
75
+ if (fbtScope === null) {
76
+ return;
77
+ }
78
+
79
+ // if the JSX element's tag was `fbt`, mark all its operands
80
+ // to ensure that they end up in the same scope as the jsx element
81
+ // itself.
82
+ for (const operand of eachReactiveValueOperand(value)) {
83
+ operand.identifier.scope = fbtScope;
84
+
85
+ // Expand the jsx element's range to account for its operands
86
+ fbtScope.range.start = makeInstructionId(
87
+ Math.min(fbtScope.range.start, operand.identifier.mutableRange.start)
88
+ );
89
+ }
90
} else if (
91
isFbtJsxExpression(this.fbtValues, value) ||
65
- (value.kind === "CallExpression" &&
66
- this.fbtValues.has(value.callee.identifier.id))
92
+ isFbtJsxChild(this.fbtValues, lvalue, value)
93
) {
94
const fbtScope = lvalue.identifier.scope;
95
if (fbtScope === null) {
@@ -80,11 +106,24 @@ class Transform extends ReactiveFunctionVisitor<void> {
106
fbtScope.range.start = makeInstructionId(
107
Math.min(fbtScope.range.start, operand.identifier.mutableRange.start)
108
);
109
+
110
+ // NOTE: we add the operands as fbt values so that they are also
111
+ // grouped with this expression
112
+ this.fbtValues.add(operand.identifier.id);
113
}
114
}
115
}
116
}
117
118
+function isFbtCallExpression(
119
+ fbtValues: Set<IdentifierId>,
120
+ value: ReactiveValue
121
+): boolean {
122
+ return (
123
+ value.kind === "CallExpression" && fbtValues.has(value.callee.identifier.id)
124
+ );
125
+}
126
+
127
function isFbtJsxExpression(
128
fbtValues: Set<IdentifierId>,
129
value: ReactiveValue
@@ -93,6 +132,18 @@ function isFbtJsxExpression(
132
value.kind === "JsxExpression" &&
133
((value.tag.kind === "Identifier" &&
134
fbtValues.has(value.tag.identifier.id)) ||
96
- (value.tag.kind === "BuiltinTag" && value.tag.name === "fbt"))
135
+ (value.tag.kind === "BuiltinTag" && FBT_TAGS.has(value.tag.name)))
136
+ );
137
+}
138
+
139
+function isFbtJsxChild(
140
+ fbtValues: Set<IdentifierId>,
141
+ lvalue: Place | null,
142
+ value: ReactiveValue
143
+): boolean {
144
+ return (
145
+ (value.kind === "JsxExpression" || value.kind === "JsxFragment") &&
146
+ lvalue !== null &&
147
+ fbtValues.has(lvalue.identifier.id)
148
);
149
}
compiler/packages/babel-plugin-react-forget/src/Utils/logger.ts
+32
-1
@@ -5,10 +5,13 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
+import generate from "@babel/generator";
9
+import * as t from "@babel/types";
10
import chalk from "chalk";
11
+import { format } from "prettier";
12
import { HIR, HIRFunction, ReactiveFunction } from "../HIR/HIR";
13
import { printFunction, printHIR } from "../HIR/PrintHIR";
11
-import { printReactiveFunction } from "../ReactiveScopes";
14
+import { CodegenFunction, printReactiveFunction } from "../ReactiveScopes";
15
16
let ENABLED: boolean = false;
17
@@ -30,6 +33,34 @@ export function logHIR(step: string, ir: HIR): void {
33
}
34
}
35
36
+export function logCodegenFunction(step: string, fn: CodegenFunction): void {
37
+ if (ENABLED) {
38
+ let printed: string | null = null;
39
+ try {
40
+ const node = t.functionDeclaration(
41
+ fn.id,
42
+ fn.params,
43
+ fn.body,
44
+ fn.generator,
45
+ fn.async
46
+ );
47
+ const ast = generate(node);
48
+ printed = format(ast.code);
49
+ } catch (e) {
50
+ console.log("Error formatting AST: " + e.message);
51
+ }
52
+ if (printed === null) {
53
+ return;
54
+ }
55
+ if (printed !== lastLogged) {
56
+ lastLogged = printed;
57
+ process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`);
58
+ } else {
59
+ process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`);
60
+ }
61
+ }
62
+}
63
+
64
export function logHIRFunction(step: string, fn: HIRFunction): void {
65
if (ENABLED) {
66
const printed = printFunction(fn);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.expect.md
+10
-17
@@ -21,28 +21,21 @@ import { unstable_useMemoCache as useMemoCache } from "react";
21
import fbt from "fbt";
22
23
function Component(props) {
24
- const $ = useMemoCache(4);
24
+ const $ = useMemoCache(2);
25
const c_0 = $[0] !== props.name;
26
- let t1;
26
+ let t0;
27
if (c_0) {
28
- const c_2 = $[2] !== props.name;
29
- let t0;
30
- if (c_2) {
31
- t0 = capitalize(props.name);
32
- $[2] = props.name;
33
- $[3] = t0;
34
- } else {
35
- t0 = $[3];
36
- }
37
- t1 = fbt._("Hello {user name}", [fbt._param("user name", t0)], {
38
- hk: "2zEDKF",
39
- });
28
+ t0 = fbt._(
29
+ "Hello {user name}",
30
+ [fbt._param("user name", capitalize(props.name))],
31
+ { hk: "2zEDKF" }
32
+ );
33
$[0] = props.name;
41
- $[1] = t1;
34
+ $[1] = t0;
35
} else {
43
- t1 = $[1];
36
+ t0 = $[1];
37
}
45
- return t1;
38
+ return t0;
39
}
40
41
```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.expect.md
new
+84
@@ -0,0 +1,84 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @debug
6
+import fbt from "fbt";
7
+
8
+function Component({ name, data, icon }) {
9
+ return (
10
+ <Text type="body4">
11
+ <fbt desc="Lorem ipsum">
12
+ <fbt:param name="item author">
13
+ <Text type="h4">{name}</Text>
14
+ </fbt:param>
15
+ <fbt:param name="icon">{icon}</fbt:param>
16
+ <Text type="h4">
17
+ <fbt:param name="item details">{data}</fbt:param>
18
+ </Text>
19
+ </fbt>
20
+ </Text>
21
+ );
22
+}
23
+
24
+```
25
+
26
+## Code
27
+
28
+```javascript
29
+import { unstable_useMemoCache as useMemoCache } from "react"; // @debug
30
+import fbt from "fbt";
31
+
32
+function Component(t39) {
33
+ const $ = useMemoCache(6);
34
+ const { name, data, icon } = t39;
35
+ const c_0 = $[0] !== name;
36
+ const c_1 = $[1] !== icon;
37
+ const c_2 = $[2] !== data;
38
+ let t0;
39
+ if (c_0 || c_1 || c_2) {
40
+ t0 = fbt._(
41
+ "{item author}{icon}{=m2}",
42
+ [
43
+ fbt._param(
44
+ "item author",
45
+
46
+ <Text type="h4">{name}</Text>
47
+ ),
48
+ fbt._param(
49
+ "icon",
50
+
51
+ icon
52
+ ),
53
+ fbt._implicitParam(
54
+ "=m2",
55
+ <Text type="h4">
56
+ {fbt._("{item details}", [fbt._param("item details", data)], {
57
+ hk: "4jLfVq",
58
+ })}
59
+ </Text>
60
+ ),
61
+ ],
62
+ { hk: "2HLm2j" }
63
+ );
64
+ $[0] = name;
65
+ $[1] = icon;
66
+ $[2] = data;
67
+ $[3] = t0;
68
+ } else {
69
+ t0 = $[3];
70
+ }
71
+ const c_4 = $[4] !== t0;
72
+ let t1;
73
+ if (c_4) {
74
+ t1 = <Text type="body4">{t0}</Text>;
75
+ $[4] = t0;
76
+ $[5] = t1;
77
+ } else {
78
+ t1 = $[5];
79
+ }
80
+ return t1;
81
+}
82
+
83
+```
84
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.js
new
+18
@@ -0,0 +1,18 @@
1
+// @debug
2
+import fbt from "fbt";
3
+
4
+function Component({ name, data, icon }) {
5
+ return (
6
+ <Text type="body4">
7
+ <fbt desc="Lorem ipsum">
8
+ <fbt:param name="item author">
9
+ <Text type="h4">{name}</Text>
10
+ </fbt:param>
11
+ <fbt:param name="icon">{icon}</fbt:param>
12
+ <Text type="h4">
13
+ <fbt:param name="item details">{data}</fbt:param>
14
+ </Text>
15
+ </fbt>
16
+ </Text>
17
+ );
18
+}
compiler/packages/sprout/src/SproutTodoFilter.ts
+1
@@ -454,6 +454,7 @@ const skipFilter = new Set([
454
"infer-function-expression-React-memo-gating",
455
"infer-skip-components-without-hooks-or-jsx",
456
"class-component-with-render-helper",
457
+ "fbtparam-with-jsx-element-content",
458
]);
459
460
export default skipFilter;