Special-case fbt() function call form
Similar to what we did for `<fbt>` jsx elements, this PR ensures that `fbt()` calls have their operands memoized in the same scope to honor the limited contract for what's allowed as an argument of an fbt() call expression.
Joe Savona committed
Apr 3, 2023 at 11:47 UTC
e32ea49a0e8590a9a5ff62bc50409dc547e98ce8
5 files changed
+128
-11
compiler/forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts
+22
-11
@@ -15,21 +15,27 @@ import { eachInstructionValueOperand } from "../HIR/visitors";
15
import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
16
17
/**
18
- * This is a Meta-ism. We special-case the `<fbt>` element for translation purposes,
19
- * and have a transform that requires the children of this element to be a limited
20
- * subset of nodes. Notably, any dynamic translation values must appear as
21
- * `<fbt:param>` children — we disallow identifiers as children of `<fbt>` nodes.
18
+ * This pass supports the `fbt` translation system (https://facebook.github.io/fbt/).
19
+ * FBT provides the `<fbt>` JSX element and `fbt()` calls (which take params in the
20
+ * form of `<fbt:param>` children or `fbt.param()` arguments, respectively). These
21
+ * tags/functions have restrictions on what types of syntax may appear as props/children/
22
+ * arguments, notably that variable references may not appear directly — variables
23
+ * must always be wrapped in a `<fbt:param>` or `fbt.param()`.
24
*
23
- * This PR adds a new pass which finds `<fbt>` nodes and ensures their immediate
24
- * operands are not independently memoized. Note that this still allows the values
25
- * of `<fbt:param>` to be independently memoized
25
+ * To ensure that Forget doesn't rewrite code to violate this restriction, we force
26
+ * operands to fbt tags/calls have the same scope as the tag/call itself.
27
+ *
28
+ * Note that this still allows the props/arguments of `<fbt:param>`/`fbt.param()`
29
+ * to be independently memoized
30
*/
31
export function memoizeFbtOperandsInSameScope(fn: ReactiveFunction): void {
32
visitReactiveFunction(fn, new Transform(), undefined);
33
}
34
35
class Transform extends ReactiveFunctionVisitor<void> {
32
- fbtTags: Set<IdentifierId> = new Set();
36
+ // Values that represent *potential* references of `fbt` as a JSX tag name
37
+ // or as a callee.
38
+ fbtValues: Set<IdentifierId> = new Set();
39
40
override visitInstruction(
41
instruction: ReactiveInstruction,
@@ -46,10 +52,15 @@ class Transform extends ReactiveFunctionVisitor<void> {
52
) {
53
// We don't distinguish between tag names and strings, so record
54
// all `fbt` string literals in case they are used as a jsx tag.
49
- this.fbtTags.add(lvalue.identifier.id);
55
+ this.fbtValues.add(lvalue.identifier.id);
56
+ } else if (value.kind === "LoadGlobal" && value.name === "fbt") {
57
+ // Record references to `fbt` as a global
58
+ this.fbtValues.add(lvalue.identifier.id);
59
} else if (
51
- value.kind === "JsxExpression" &&
52
- this.fbtTags.has(value.tag.identifier.id)
60
+ (value.kind === "JsxExpression" &&
61
+ this.fbtValues.has(value.tag.identifier.id)) ||
62
+ (value.kind === "CallExpression" &&
63
+ this.fbtValues.has(value.callee.identifier.id))
64
) {
65
// if the JSX element's tag was `fbt`, mark all its operands
66
// to ensure that they end up in the same scope as the jsx element
compiler/forget/src/__tests__/fixtures/compiler/fbt-call-complex-param-value.expect.md
new
+46
@@ -0,0 +1,46 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const text = fbt(
7
+ `Hello, ${fbt.param("(key) name", capitalize(props.name))}!`,
8
+ "(description) Greeting"
9
+ );
10
+ return <div>{text}</div>;
11
+}
12
+
13
+```
14
+
15
+## Code
16
+
17
+```javascript
18
+function Component(props) {
19
+ const $ = React.unstable_useMemoCache(4);
20
+ const c_0 = $[0] !== props.name;
21
+ let t0;
22
+ if (c_0) {
23
+ t0 = fbt(
24
+ `Hello, ${fbt.param("(key) name", capitalize(props.name))}!`,
25
+ "(description) Greeting"
26
+ );
27
+ $[0] = props.name;
28
+ $[1] = t0;
29
+ } else {
30
+ t0 = $[1];
31
+ }
32
+ const text = t0;
33
+ const c_2 = $[2] !== text;
34
+ let t1;
35
+ if (c_2) {
36
+ t1 = <div>{text}</div>;
37
+ $[2] = text;
38
+ $[3] = t1;
39
+ } else {
40
+ t1 = $[3];
41
+ }
42
+ return t1;
43
+}
44
+
45
+```
46
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/fbt-call-complex-param-value.js
new
+7
@@ -0,0 +1,7 @@
1
+function Component(props) {
2
+ const text = fbt(
3
+ `Hello, ${fbt.param("(key) name", capitalize(props.name))}!`,
4
+ "(description) Greeting"
5
+ );
6
+ return <div>{text}</div>;
7
+}
compiler/forget/src/__tests__/fixtures/compiler/fbt-call.expect.md
new
+46
@@ -0,0 +1,46 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const text = fbt(
7
+ `${fbt.param("(key) count", props.count)} items`,
8
+ "(description) Number of items"
9
+ );
10
+ return <div>{text}</div>;
11
+}
12
+
13
+```
14
+
15
+## Code
16
+
17
+```javascript
18
+function Component(props) {
19
+ const $ = React.unstable_useMemoCache(4);
20
+ const c_0 = $[0] !== props.count;
21
+ let t0;
22
+ if (c_0) {
23
+ t0 = fbt(
24
+ `${fbt.param("(key) count", props.count)} items`,
25
+ "(description) Number of items"
26
+ );
27
+ $[0] = props.count;
28
+ $[1] = t0;
29
+ } else {
30
+ t0 = $[1];
31
+ }
32
+ const text = t0;
33
+ const c_2 = $[2] !== text;
34
+ let t1;
35
+ if (c_2) {
36
+ t1 = <div>{text}</div>;
37
+ $[2] = text;
38
+ $[3] = t1;
39
+ } else {
40
+ t1 = $[3];
41
+ }
42
+ return t1;
43
+}
44
+
45
+```
46
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/fbt-call.js
new
+7
@@ -0,0 +1,7 @@
1
+function Component(props) {
2
+ const text = fbt(
3
+ `${fbt.param("(key) count", props.count)} items`,
4
+ "(description) Number of items"
5
+ );
6
+ return <div>{text}</div>;
7
+}