@samitouri / QOS-React-2 / commits / 4e2ef92779

Ensure <fbt> children are not independently memod

This is a Meta-ism, but adding it for now to unblock. We special-case the `<fbt>` element for translation purposes, and have a transform that requires the children of this element to be a limited subset of nodes. Notably, any dynamic translation values must appear as `<fbt:param>` children — we disallow identifiers as children of `<fbt>` nodes. This PR adds a new pass which finds `<fbt>` nodes and ensures their immediate operands are not independently memoized. Note that this still allows the values of `<fbt:param>` to be independently memoized, as demonstrated in the unit test.

Joe Savona committed Mar 31, 2023 at 12:32 UTC 4e2ef92779bd4e55583bd3be1d88399cb4c883bb
9 files changed +208 -21
compiler/forget/src/CompilerPipeline.ts
+9 -1
@@ -12,7 +12,7 @@ import {
12 mergeConsecutiveBlocks,
13 ReactiveFunction,
14 } from "./HIR";
15 -import { EnvironmentConfig, Environment } from "./HIR/Environment";
15 +import { Environment, EnvironmentConfig } from "./HIR/Environment";
16 import { validateConsistentIdentifiers } from "./HIR/ValidateConsistentIdentifiers";
17 import {
18 analyseFunctions,
@@ -29,6 +29,7 @@ import {
29 flattenReactiveLoops,
30 flattenScopesWithHooks,
31 inferReactiveScopeVariables,
32 + memoizeFbtOperandsInSameScope,
33 mergeOverlappingReactiveScopes,
34 promoteUsedTemporaries,
35 propagateScopeDependencies,
@@ -104,6 +105,13 @@ export function* run(
105 value: reactiveFunction,
106 });
107
108 + memoizeFbtOperandsInSameScope(reactiveFunction);
109 + yield log({
110 + kind: "reactive",
111 + name: "MemoizeFbtOperandsInSameScope",
112 + value: reactiveFunction,
113 + });
114 +
115 alignReactiveScopesToBlockScopes(reactiveFunction);
116 yield log({
117 kind: "reactive",
compiler/forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts new
+72
@@ -0,0 +1,72 @@
1 +/**
2 + * Copyright (c) Facebook, Inc. and its affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + */
7 +
8 +import {
9 + IdentifierId,
10 + makeInstructionId,
11 + ReactiveFunction,
12 + ReactiveInstruction,
13 +} from "../HIR";
14 +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.
22 + *
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
26 + */
27 +export function memoizeFbtOperandsInSameScope(fn: ReactiveFunction): void {
28 + visitReactiveFunction(fn, new Transform(), undefined);
29 +}
30 +
31 +class Transform extends ReactiveFunctionVisitor<void> {
32 + fbtTags: Set<IdentifierId> = new Set();
33 +
34 + override visitInstruction(
35 + instruction: ReactiveInstruction,
36 + _state: void
37 + ): void {
38 + const { lvalue, value } = instruction;
39 + if (lvalue === null) {
40 + return;
41 + }
42 + if (
43 + value.kind === "Primitive" &&
44 + typeof value.value === "string" &&
45 + value.value === "fbt"
46 + ) {
47 + // We don't distinguish between tag names and strings, so record
48 + // all `fbt` string literals in case they are used as a jsx tag.
49 + this.fbtTags.add(lvalue.identifier.id);
50 + } else if (
51 + value.kind === "JsxExpression" &&
52 + this.fbtTags.has(value.tag.identifier.id)
53 + ) {
54 + // if the JSX element's tag was `fbt`, mark all its operands
55 + // to ensure that they end up in the same scope as the jsx element
56 + // itself.
57 + for (const operand of eachInstructionValueOperand(value)) {
58 + operand.identifier.scope = lvalue.identifier.scope;
59 + operand.identifier.mutableRange.end =
60 + lvalue.identifier.mutableRange.end;
61 +
62 + // Expand the jsx element's range to account for its operands
63 + lvalue.identifier.mutableRange.start = makeInstructionId(
64 + Math.min(
65 + lvalue.identifier.mutableRange.start,
66 + operand.identifier.mutableRange.start
67 + )
68 + );
69 + }
70 + }
71 + }
72 +}
compiler/forget/src/ReactiveScopes/index.ts
+1
@@ -12,6 +12,7 @@ export { codegenReactiveFunction } from "./CodegenReactiveFunction";
12 export { flattenReactiveLoops } from "./FlattenReactiveLoops";
13 export { flattenScopesWithHooks } from "./FlattenScopesWithHooks";
14 export { inferReactiveScopeVariables } from "./InferReactiveScopeVariables";
15 +export { memoizeFbtOperandsInSameScope } from "./MemoizeFbtOperandsInSameScope";
16 export { mergeOverlappingReactiveScopes } from "./MergeOverlappingReactiveScopes";
17 export { printReactiveFunction } from "./PrintReactiveFunction";
18 export { promoteUsedTemporaries } from "./PromoteUsedTemporaries";
compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.expect.md new
+46
@@ -0,0 +1,46 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + return (
7 + <fbt desc={"Dialog to show to user"}>
8 + Hello <fbt:param name="user name">{capitalize(props.name)}</fbt:param>
9 + </fbt>
10 + );
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 t1;
22 + if (c_0) {
23 + const c_2 = $[2] !== props.name;
24 + let t0;
25 + if (c_2) {
26 + t0 = capitalize(props.name);
27 + $[2] = props.name;
28 + $[3] = t0;
29 + } else {
30 + t0 = $[3];
31 + }
32 + t1 = (
33 + <fbt desc={"Dialog to show to user"}>
34 + Hello {<fbt:param name={"user name"}>{t0}</fbt:param>}
35 + </fbt>
36 + );
37 + $[0] = props.name;
38 + $[1] = t1;
39 + } else {
40 + t1 = $[1];
41 + }
42 + return t1;
43 +}
44 +
45 +```
46 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.js new
+7
@@ -0,0 +1,7 @@
1 +function Component(props) {
2 + return (
3 + <fbt desc={"Dialog to show to user"}>
4 + Hello <fbt:param name="user name">{capitalize(props.name)}</fbt:param>
5 + </fbt>
6 + );
7 +}
compiler/forget/src/__tests__/fixtures/compiler/fbt-params.expect.md new
+37
@@ -0,0 +1,37 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + return (
7 + <fbt desc={"Dialog to show to user"}>
8 + Hello <fbt:param name="user name">{props.name}</fbt:param>
9 + </fbt>
10 + );
11 +}
12 +
13 +```
14 +
15 +## Code
16 +
17 +```javascript
18 +function Component(props) {
19 + const $ = React.unstable_useMemoCache(2);
20 + const c_0 = $[0] !== props.name;
21 + let t0;
22 + if (c_0) {
23 + t0 = (
24 + <fbt desc={"Dialog to show to user"}>
25 + Hello {<fbt:param name={"user name"}>{props.name}</fbt:param>}
26 + </fbt>
27 + );
28 + $[0] = props.name;
29 + $[1] = t0;
30 + } else {
31 + t0 = $[1];
32 + }
33 + return t0;
34 +}
35 +
36 +```
37 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/fbt-params.js new
+7
@@ -0,0 +1,7 @@
1 +function Component(props) {
2 + return (
3 + <fbt desc={"Dialog to show to user"}>
4 + Hello <fbt:param name="user name">{props.name}</fbt:param>
5 + </fbt>
6 + );
7 +}
compiler/forget/src/index.ts
+2
@@ -18,5 +18,7 @@ declare global {
18 let __DEV__: boolean | null | undefined;
19 }
20
21 +console.log("loading Forget!!!");
22 +
23 import ReactForgetBabelPlugin from "./Babel/BabelPlugin";
24 export default ReactForgetBabelPlugin;
compiler/forget/yarn.lock
+27 -20
@@ -17,6 +17,13 @@
17 dependencies:
18 "@babel/highlight" "^7.18.6"
19
20 +"@babel/code-frame@^7.21.4":
21 + version "7.21.4"
22 + resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.21.4.tgz#d0fa9e4413aca81f2b23b9442797bda1826edb39"
23 + integrity sha512-LYvhNKfwWSPpocw8GI7gpK2nq3HSDuEPC/uSYaALSJu9xjsalaaYFOq0Pwt5KmVqwEbZlDu81aLXwBOmD/Fv9g==
24 + dependencies:
25 + "@babel/highlight" "^7.18.6"
26 +
27 "@babel/compat-data@^7.19.1":
28 version "7.19.1"
29 resolved "https://registry.yarnpkg.com/@babel/compat-data/-/compat-data-7.19.1.tgz#72d647b4ff6a4f82878d184613353af1dd0290f9"
@@ -83,12 +90,12 @@
90 "@jridgewell/gen-mapping" "^0.3.2"
91 jsesc "^2.5.1"
92
86 -"@babel/generator@^7.21.3":
87 - version "7.21.3"
88 - resolved "https://registry.yarnpkg.com/@babel/generator/-/generator-7.21.3.tgz#232359d0874b392df04045d72ce2fd9bb5045fce"
89 - integrity sha512-QS3iR1GYC/YGUnW7IdggFeN5c1poPUurnGttOV/bZgPGV+izC/D8HnD6DLwod0fsatNyVn1G3EVWMYIF0nHbeA==
93 +"@babel/generator@^7.21.4":
94 + version "7.21.4"
95 + resolved "https://registry.yarnpkg.com/@babel/generator/-/generator-7.21.4.tgz#64a94b7448989f421f919d5239ef553b37bb26bc"
96 + integrity sha512-NieM3pVIYW2SwGzKoqfPrQsf4xGs9M9AIG3ThppsSRmO+m7eQhmI6amajKMUeIO37wFfsvnvcxQFx6x6iqxDnA==
97 dependencies:
91 - "@babel/types" "^7.21.3"
98 + "@babel/types" "^7.21.4"
99 "@jridgewell/gen-mapping" "^0.3.2"
100 "@jridgewell/trace-mapping" "^0.3.17"
101 jsesc "^2.5.1"
@@ -274,10 +281,10 @@
281 resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.2.tgz#dacafadfc6d7654c3051a66d6fe55b6cb2f2a0b3"
282 integrity sha512-URpaIJQwEkEC2T9Kn+Ai6Xe/02iNaVCuT/PtoRz3GPVJVDpPd7mLo+VddTbhCRU9TXqW5mSrQfXZyi8kDKOVpQ==
283
277 -"@babel/parser@^7.21.3":
278 - version "7.21.3"
279 - resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.3.tgz#1d285d67a19162ff9daa358d4cb41d50c06220b3"
280 - integrity sha512-lobG0d7aOfQRXh8AyklEAgZGvA4FShxo6xQbUrrT/cNBPUdIDojlokwJsQyCC/eKia7ifqM0yP+2DRZ4WKw2RQ==
284 +"@babel/parser@^7.21.4":
285 + version "7.21.4"
286 + resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.4.tgz#94003fdfc520bbe2875d4ae557b43ddb6d880f17"
287 + integrity sha512-alVJj7k7zIxqBZ7BTRhz0IqJFxW1VJbm6N8JbcYhQ186df9ZBPbZBmWSqAMXwHGsCJdYks7z/voa3ibiS5bCIw==
288
289 "@babel/plugin-syntax-async-generators@^7.8.4":
290 version "7.8.4"
@@ -507,18 +514,18 @@
514 lodash "^4.17.10"
515
516 "@babel/traverse@^7.19.1":
510 - version "7.21.3"
511 - resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.21.3.tgz#4747c5e7903d224be71f90788b06798331896f67"
512 - integrity sha512-XLyopNeaTancVitYZe2MlUEvgKb6YVVPXzofHgqHijCImG33b/uTurMS488ht/Hbsb2XK3U2BnSTxKVNGV3nGQ==
517 + version "7.21.4"
518 + resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.21.4.tgz#a836aca7b116634e97a6ed99976236b3282c9d36"
519 + integrity sha512-eyKrRHKdyZxqDm+fV1iqL9UAHMoIg0nDaGqfIOd8rKH17m5snv7Gn4qgjBoFfLz9APvjFU/ICT00NVCv1Epp8Q==
520 dependencies:
514 - "@babel/code-frame" "^7.18.6"
515 - "@babel/generator" "^7.21.3"
521 + "@babel/code-frame" "^7.21.4"
522 + "@babel/generator" "^7.21.4"
523 "@babel/helper-environment-visitor" "^7.18.9"
524 "@babel/helper-function-name" "^7.21.0"
525 "@babel/helper-hoist-variables" "^7.18.6"
526 "@babel/helper-split-export-declaration" "^7.18.6"
520 - "@babel/parser" "^7.21.3"
521 - "@babel/types" "^7.21.3"
527 + "@babel/parser" "^7.21.4"
528 + "@babel/types" "^7.21.4"
529 debug "^4.1.0"
530 globals "^11.1.0"
531
@@ -575,10 +582,10 @@
582 "@babel/helper-validator-identifier" "^7.19.1"
583 to-fast-properties "^2.0.0"
584
578 -"@babel/types@^7.21.3":
579 - version "7.21.3"
580 - resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.21.3.tgz#4865a5357ce40f64e3400b0f3b737dc6d4f64d05"
581 - integrity sha512-sBGdETxC+/M4o/zKC0sl6sjWv62WFR/uzxrJ6uYyMLZOUlPnwzw0tKgVHOXxaAd5l2g8pEDM5RZ495GPQI77kg==
585 +"@babel/types@^7.21.4":
586 + version "7.21.4"
587 + resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.21.4.tgz#2d5d6bb7908699b3b416409ffd3b5daa25b030d4"
588 + integrity sha512-rU2oY501qDxE8Pyo7i/Orqma4ziCOrby0/9mvbDUGEfvZjb279Nk9k19e2fiCxHbRRpY2ZyrgW1eq22mvmOIzA==
589 dependencies:
590 "@babel/helper-string-parser" "^7.19.4"
591 "@babel/helper-validator-identifier" "^7.19.1"