@samitouri / QOS-React-2 / commits / 1e2dbf7fdb

Fix JSX form of fbt

Fixes `<fbt>`. This required a bunk of yak shaving to work through several issues: * First, there was a bug in codegen for JsxNamedspacedName. I added handling for it for identifiers, but JsxNamespacedName gets converted to a Primitive. The output looked correct because Babel happily creates invalid Jsx identifiers! * Next, I needed to add locations to JSX nodes. It took me a while to pinpoint which specific node needed the location, so I ended up just adding locations to all the parts of a Jsx element. * That uncovered the fact that FBT was expecting the `<fbt:param>`'s `name` attribute value to be a StringLiteral, not a StringLiteral wrapped in a JsxExpressionContainer. So now we special-case JsxAttribute and emit raw StringLiteral (either is allowed per the spec) And with that, voila, `<fbt>` works.

Joe Savona committed Apr 4, 2023 at 14:37 UTC 1e2dbf7fdbbaca335297568f17be984709ab2b28
33 files changed +179 -125
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+80 -51
@@ -14,6 +14,7 @@ import {
14 Identifier,
15 IdentifierId,
16 InstructionKind,
17 + JsxAttribute,
18 Pattern,
19 Place,
20 ReactiveBlock,
@@ -566,6 +567,14 @@ const createLogicalExpression = withLoc(t.logicalExpression);
567 const createSequenceExpression = withLoc(t.sequenceExpression);
568 const createConditionalExpression = withLoc(t.conditionalExpression);
569 const createTemplateLiteral = withLoc(t.templateLiteral);
570 +const createJsxNamespacedName = withLoc(t.jsxNamespacedName);
571 +const createJsxElement = withLoc(t.jsxElement);
572 +const createJsxAttribute = withLoc(t.jsxAttribute);
573 +const createJsxIdentifier = withLoc(t.jsxIdentifier);
574 +const createJsxExpressionContainer = withLoc(t.jsxExpressionContainer);
575 +const createJsxText = withLoc(t.jsxText);
576 +const createJsxClosingElement = withLoc(t.jsxClosingElement);
577 +const createStringLiteral = withLoc(t.stringLiteral);
578
579 type Temporaries = Map<IdentifierId, t.Expression>;
580
@@ -642,7 +651,7 @@ function codegenInstructionValue(
651 break;
652 }
653 case "Primitive": {
645 - value = codegenValue(cx, instrValue.value);
654 + value = codegenValue(cx, instrValue.loc, instrValue.value);
655 break;
656 }
657 case "CallExpression": {
@@ -712,58 +721,18 @@ function codegenInstructionValue(
721 break;
722 }
723 case "JSXText": {
715 - value = t.stringLiteral(instrValue.value);
724 + value = createStringLiteral(instrValue.loc, instrValue.value);
725 break;
726 }
727 case "JsxExpression": {
728 const attributes: Array<t.JSXAttribute | t.JSXSpreadAttribute> = [];
729 for (const attribute of instrValue.props) {
721 - switch (attribute.kind) {
722 - case "JsxAttribute": {
723 - let propName: t.JSXIdentifier | t.JSXNamespacedName;
724 - if (attribute.name.indexOf(":") === -1) {
725 - propName = t.jsxIdentifier(attribute.name);
726 - } else {
727 - const [namespace, name] = attribute.name.split(":", 2);
728 - propName = t.jsxNamespacedName(
729 - t.jsxIdentifier(namespace),
730 - t.jsxIdentifier(name)
731 - );
732 - }
733 - attributes.push(
734 - t.jsxAttribute(
735 - propName,
736 - t.jsxExpressionContainer(codegenPlace(cx, attribute.place))
737 - )
738 - );
739 - break;
740 - }
741 - case "JsxSpreadAttribute": {
742 - attributes.push(
743 - t.jsxSpreadAttribute(codegenPlace(cx, attribute.argument))
744 - );
745 - break;
746 - }
747 - default: {
748 - assertExhaustive(
749 - attribute,
750 - `Unexpected attribute kind '${(attribute as any).kind}'`
751 - );
752 - }
753 - }
730 + attributes.push(codegenJsxAttribute(cx, attribute));
731 }
732 let tagValue = codegenPlace(cx, instrValue.tag);
733 let tag: t.JSXIdentifier | t.JSXNamespacedName | t.JSXMemberExpression;
734 if (tagValue.type === "Identifier") {
758 - if (tagValue.name.indexOf(":") >= 0) {
759 - const [namespace, name] = tagValue.name.split(":", 2);
760 - tag = t.jsxNamespacedName(
761 - t.jsxIdentifier(namespace),
762 - t.jsxIdentifier(name)
763 - );
764 - } else {
765 - tag = t.jsxIdentifier(tagValue.name);
766 - }
735 + tag = createJsxIdentifier(instrValue.tag.loc, tagValue.name);
736 } else if (tagValue.type === "MemberExpression") {
737 tag = convertMemberExpressionToJsx(tagValue);
738 } else {
@@ -772,15 +741,27 @@ function codegenInstructionValue(
741 "Expected JSX tag to be an identifier or string, got '%s'",
742 tagValue.type
743 );
775 - tag = t.jsxIdentifier(tagValue.value);
744 + if (tagValue.value.indexOf(":") >= 0) {
745 + const [namespace, name] = tagValue.value.split(":", 2);
746 + tag = createJsxNamespacedName(
747 + instrValue.tag.loc,
748 + createJsxIdentifier(instrValue.tag.loc, namespace),
749 + createJsxIdentifier(instrValue.tag.loc, name)
750 + );
751 + } else {
752 + tag = createJsxIdentifier(instrValue.loc, tagValue.value);
753 + }
754 }
755 const children =
756 instrValue.children !== null
757 ? instrValue.children.map((child) => codegenJsxElement(cx, child))
758 : [];
781 - value = t.jsxElement(
759 + value = createJsxElement(
760 + instrValue.loc,
761 t.jsxOpeningElement(tag, attributes, instrValue.children === null),
783 - instrValue.children !== null ? t.jsxClosingElement(tag) : null,
762 + instrValue.children !== null
763 + ? createJsxClosingElement(instrValue.tag.loc, tag)
764 + : null,
765 children,
766 instrValue.children === null
767 );
@@ -1007,6 +988,51 @@ function codegenInstructionValue(
988 return value;
989 }
990
991 +function codegenJsxAttribute(
992 + cx: Context,
993 + attribute: JsxAttribute
994 +): t.JSXAttribute | t.JSXSpreadAttribute {
995 + switch (attribute.kind) {
996 + case "JsxAttribute": {
997 + let propName: t.JSXIdentifier | t.JSXNamespacedName;
998 + if (attribute.name.indexOf(":") === -1) {
999 + propName = createJsxIdentifier(attribute.place.loc, attribute.name);
1000 + } else {
1001 + const [namespace, name] = attribute.name.split(":", 2);
1002 + propName = createJsxNamespacedName(
1003 + attribute.place.loc,
1004 + createJsxIdentifier(attribute.place.loc, namespace),
1005 + createJsxIdentifier(attribute.place.loc, name)
1006 + );
1007 + }
1008 + const innerValue = codegenPlace(cx, attribute.place);
1009 + let value;
1010 + switch (innerValue.type) {
1011 + case "StringLiteral":
1012 + case "JSXElement":
1013 + case "JSXFragment": {
1014 + value = innerValue;
1015 + break;
1016 + }
1017 + default: {
1018 + value = createJsxExpressionContainer(attribute.place.loc, innerValue);
1019 + break;
1020 + }
1021 + }
1022 + return createJsxAttribute(attribute.place.loc, propName, value);
1023 + }
1024 + case "JsxSpreadAttribute": {
1025 + return t.jsxSpreadAttribute(codegenPlace(cx, attribute.argument));
1026 + }
1027 + default: {
1028 + assertExhaustive(
1029 + attribute,
1030 + `Unexpected attribute kind '${(attribute as any).kind}'`
1031 + );
1032 + }
1033 + }
1034 +}
1035 +
1036 function codegenJsxElement(
1037 cx: Context,
1038 place: Place
@@ -1019,14 +1045,14 @@ function codegenJsxElement(
1045 const value = codegenPlace(cx, place);
1046 switch (value.type) {
1047 case "StringLiteral": {
1022 - return t.jsxText(value.value);
1048 + return createJsxText(place.loc, value.value);
1049 }
1050 case "JSXElement":
1051 case "JSXFragment": {
1052 return value;
1053 }
1054 default: {
1029 - return t.jsxExpressionContainer(value);
1055 + return createJsxExpressionContainer(place.loc, value);
1056 }
1057 }
1058 }
@@ -1093,6 +1119,7 @@ function codegenLValue(
1119
1120 function codegenValue(
1121 cx: Context,
1122 + loc: SourceLocation,
1123 value: boolean | number | string | null | undefined
1124 ): t.Expression {
1125 if (typeof value === "number") {
@@ -1100,7 +1127,7 @@ function codegenValue(
1127 } else if (typeof value === "boolean") {
1128 return t.booleanLiteral(value);
1129 } else if (typeof value === "string") {
1103 - return t.stringLiteral(value);
1130 + return createStringLiteral(loc, value);
1131 } else if (value === null) {
1132 return t.nullLiteral();
1133 } else if (value === undefined) {
@@ -1126,7 +1153,9 @@ function codegenPlace(cx: Context, place: Place): t.Expression {
1153 if (tmp != null) {
1154 return tmp;
1155 }
1129 - return convertIdentifier(place.identifier);
1156 + const identifier = convertIdentifier(place.identifier);
1157 + identifier.loc = place.loc as any;
1158 + return identifier;
1159 }
1160
1161 function convertIdentifier(identifier: Identifier): t.Identifier {
compiler/forget/src/__tests__/fixtures/compiler/alias-computed-load.expect.md
+1 -1
@@ -23,8 +23,8 @@ function component(a) {
23 if (c_0) {
24 x = { a };
25 const y = {};
26 - y.x = x.a;
26
27 + y.x = x.a;
28 mutate(y);
29 $[0] = a;
30 $[1] = x;
compiler/forget/src/__tests__/fixtures/compiler/alias-nested-member-path-mutate.expect.md
-2
@@ -24,10 +24,8 @@ function component() {
24 const z = [];
25 const y = {};
26 y.z = z;
27 -
27 x = {};
28 x.y = y;
30 -
29 mutate(x.y.z);
30 $[0] = x;
31 } else {
compiler/forget/src/__tests__/fixtures/compiler/assignment-expression-computed.expect.md
+1
@@ -21,6 +21,7 @@ function Component(props) {
21 let x;
22 if (c_0) {
23 x = [props.x];
24 +
25 x[0] = x[0] * 2;
26 x["0"] = x["0"] + 3;
27 $[0] = props.x;
compiler/forget/src/__tests__/fixtures/compiler/computed-store-alias.expect.md
-1
@@ -24,7 +24,6 @@ function component(a, b) {
24 const y = { a };
25 x = { b };
26 x.y = y;
27 -
27 mutate(x);
28 $[0] = a;
29 $[1] = b;
compiler/forget/src/__tests__/fixtures/compiler/destructuring-assignment.expect.md
-2
@@ -30,11 +30,9 @@ function foo(a, b, c) {
30 const $ = React.unstable_useMemoCache(5);
31
32 const [d, t46] = a;
33 -
33 const [t48] = t46;
34 const { e: t50 } = t48;
35 const { f: g } = t50;
37 -
36 const { l: t55, o } = b;
37 const { m: t58 } = t55;
38 const [t60] = t58;
compiler/forget/src/__tests__/fixtures/compiler/destructuring.expect.md
-2
@@ -44,7 +44,6 @@ function foo(a, b, c) {
44 d = $[2];
45 h = $[3];
46 }
47 -
47 const [t1] = t0;
48 const c_4 = $[4] !== t1;
49 let t2;
@@ -59,7 +58,6 @@ function foo(a, b, c) {
58 g = $[6];
59 }
60 const { f } = t2;
62 -
61 const { l: t51, p } = b;
62 const { m: t3 } = t51;
63 const c_7 = $[7] !== t3;
compiler/forget/src/__tests__/fixtures/compiler/error.fbt-params-complex-param-value.expect.md deleted
-27
@@ -1,27 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -import fbt from "fbt";
6 -
7 -function Component(props) {
8 - return (
9 - <fbt desc={"Dialog to show to user"}>
10 - Hello <fbt:param name="user name">{capitalize(props.name)}</fbt:param>
11 - </fbt>
12 - );
13 -}
14 -
15 -```
16 -
17 -
18 -## Error
19 -
20 -```
21 -fbt: unsupported babel node: Identifier
22 ----
23 -t0
24 ----
25 -```
26 -
27 -
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.fbt-params.expect.md deleted
-27
@@ -1,27 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -import fbt from "fbt";
6 -
7 -function Component(props) {
8 - return (
9 - <fbt desc={"Dialog to show to user"}>
10 - Hello <fbt:param name="user name">{props.name}</fbt:param>
11 - </fbt>
12 - );
13 -}
14 -
15 -```
16 -
17 -
18 -## Error
19 -
20 -```
21 -fbt: unsupported babel node: MemberExpression
22 ----
23 -props.name
24 ----
25 -```
26 -
27 -
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/escape-analysis-non-escaping-interleaved-allocating-dependency.expect.md
-1
@@ -42,7 +42,6 @@ function Component(props) {
42 b = [];
43 const c = {};
44 c.a = a;
45 -
45 b.push(props.b);
46 $[2] = a;
47 $[3] = props.b;
compiler/forget/src/__tests__/fixtures/compiler/escape-analysis-non-escaping-interleaved-allocating-nested-dependency.expect.md
-1
@@ -61,7 +61,6 @@ function Component(props) {
61 c = [];
62 const d = {};
63 d.b = b;
64 -
64 c.push(props.b);
65 $[4] = b;
66 $[5] = props.b;
compiler/forget/src/__tests__/fixtures/compiler/escape-analysis-non-escaping-interleaved-primitive-dependency.expect.md
-1
@@ -36,7 +36,6 @@ function Component(props) {
36 b = [];
37 const c = {};
38 c.a = a;
39 -
39 b.push(props.c);
40 $[0] = a;
41 $[1] = props.c;
compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.expect.md new
+48
@@ -0,0 +1,48 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import fbt from "fbt";
6 +
7 +function Component(props) {
8 + return (
9 + <fbt desc={"Dialog to show to user"}>
10 + Hello <fbt:param name="user name">{capitalize(props.name)}</fbt:param>
11 + </fbt>
12 + );
13 +}
14 +
15 +```
16 +
17 +## Code
18 +
19 +```javascript
20 +import fbt from "fbt";
21 +
22 +function Component(props) {
23 + const $ = React.unstable_useMemoCache(4);
24 + const c_0 = $[0] !== props.name;
25 + let t1;
26 + if (c_0) {
27 + const c_2 = $[2] !== props.name;
28 + let t0;
29 + if (c_2) {
30 + t0 = capitalize(props.name);
31 + $[2] = props.name;
32 + $[3] = t0;
33 + } else {
34 + t0 = $[3];
35 + }
36 + t1 = fbt._("Hello {user name}", [fbt._param("user name", t0)], {
37 + hk: "2zEDKF",
38 + });
39 + $[0] = props.name;
40 + $[1] = t1;
41 + } else {
42 + t1 = $[1];
43 + }
44 + return t1;
45 +}
46 +
47 +```
48 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/fbt-params-complex-param-value.js renamed
compiler/forget/src/__tests__/fixtures/compiler/fbt-params.expect.md new
+39
@@ -0,0 +1,39 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import fbt from "fbt";
6 +
7 +function Component(props) {
8 + return (
9 + <fbt desc={"Dialog to show to user"}>
10 + Hello <fbt:param name="user name">{props.name}</fbt:param>
11 + </fbt>
12 + );
13 +}
14 +
15 +```
16 +
17 +## Code
18 +
19 +```javascript
20 +import fbt from "fbt";
21 +
22 +function Component(props) {
23 + const $ = React.unstable_useMemoCache(2);
24 + const c_0 = $[0] !== props.name;
25 + let t0;
26 + if (c_0) {
27 + t0 = fbt._("Hello {user name}", [fbt._param("user name", props.name)], {
28 + hk: "2zEDKF",
29 + });
30 + $[0] = props.name;
31 + $[1] = t0;
32 + } else {
33 + t0 = $[1];
34 + }
35 + return t0;
36 +}
37 +
38 +```
39 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/fbt-params.js renamed
compiler/forget/src/__tests__/fixtures/compiler/mutable-lifetime-with-aliasing.expect.md
-1
@@ -48,7 +48,6 @@ function Component(props) {
48
49 const x = {};
50 x.b = b;
51 -
51 const y = mutate(x, d);
52 if (a) {
53 }
compiler/forget/src/__tests__/fixtures/compiler/obj-literal-mutated-after-if-else.expect.md
+1
@@ -31,6 +31,7 @@ function foo(a, b, c, d) {
31 } else {
32 x = { c };
33 }
34 +
35 x.f = 1;
36 $[0] = b;
37 $[1] = c;
compiler/forget/src/__tests__/fixtures/compiler/obj-mutated-after-if-else-with-alias.expect.md
+1
@@ -35,6 +35,7 @@ function foo(a, b, c, d) {
35 } else {
36 x = someObj();
37 }
38 +
39 x.f = 1;
40 $[0] = a;
41 $[1] = x;
compiler/forget/src/__tests__/fixtures/compiler/obj-mutated-after-if-else.expect.md
+1
@@ -31,6 +31,7 @@ function foo(a, b, c, d) {
31 } else {
32 x = someObj();
33 }
34 +
35 x.f = 1;
36 $[0] = a;
37 $[1] = x;
compiler/forget/src/__tests__/fixtures/compiler/obj-mutated-after-nested-if-else-with-alias.expect.md
+1
@@ -48,6 +48,7 @@ function foo(a, b, c, d) {
48 } else {
49 x = someObj();
50 }
51 +
52 x.f = 1;
53 $[0] = a;
54 $[1] = b;
compiler/forget/src/__tests__/fixtures/compiler/object-properties.expect.md
-1
@@ -18,7 +18,6 @@ function foo(a, b, c) {
18 function foo(a, b, c) {
19 const y = b.c.d;
20 y.z = c.d.e;
21 -
21 foo(a.b.c);
22 }
23
compiler/forget/src/__tests__/fixtures/compiler/property-assignment.expect.md
-1
@@ -25,7 +25,6 @@ function Component(props) {
25 x = {};
26 const y = [];
27 x.y = y;
28 -
28 child = <Component data={y} />;
29 x.y.push(props.p0);
30 $[0] = props.p0;
compiler/forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-subpath-order2.expect.md
+1
@@ -36,6 +36,7 @@ function TestConditionalSubpath2(props, other) {
36 if (foo(other)) {
37 x.a = props.a;
38 }
39 +
40 x.b = props.a.b;
41 $[0] = other;
42 $[1] = props.a;
compiler/forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-superpath-order2.expect.md
+1
@@ -34,6 +34,7 @@ function TestConditionalSuperpath2(props, other) {
34 if (foo(other)) {
35 x.b = props.a.b;
36 }
37 +
38 x.a = props.a;
39 $[0] = other;
40 $[1] = props.a;
compiler/forget/src/__tests__/fixtures/compiler/ssa-property-alias-mutate-inside-if.expect.md
-1
@@ -29,7 +29,6 @@ function foo(a) {
29 if (a) {
30 const y = {};
31 x.y = y;
32 -
32 mutate(y);
33 } else {
34 let t0;
compiler/forget/src/__tests__/fixtures/compiler/ssa-property-mutate-2.expect.md
-1
@@ -22,7 +22,6 @@ function foo() {
22 const x = [];
23 y = {};
24 y.x = x;
25 -
25 mutate(x);
26 $[0] = y;
27 } else {
compiler/forget/src/__tests__/fixtures/compiler/ssa-property-mutate-alias.expect.md
+1
@@ -25,6 +25,7 @@ function foo() {
25 const a = {};
26 y = a;
27 const x = [];
28 +
29 y.x = x;
30
31 mutate(a);
compiler/forget/src/__tests__/fixtures/compiler/ssa-property-mutate.expect.md
-1
@@ -22,7 +22,6 @@ function foo() {
22 const x = [];
23 y = {};
24 y.x = x;
25 -
25 mutate(y);
26 $[0] = y;
27 } else {
compiler/forget/src/__tests__/fixtures/compiler/transitive-alias-fields.expect.md
+1
@@ -25,6 +25,7 @@ function component() {
25 const p = {};
26 const q = {};
27 const y = {};
28 +
29 x.y = y;
30 p.y = x.y;
31 q.y = p.y;
compiler/forget/src/__tests__/fixtures/compiler/type-cast-expression.flow.expect.md
-1
@@ -31,7 +31,6 @@ function Component(props) {
31 } else {
32 y = $[1];
33 }
34 -
34 const z = (y: Foo);
35 return z;
36 }
compiler/forget/src/__tests__/fixtures/compiler/type-test-field-store.expect.md
-1
@@ -33,7 +33,6 @@ function component() {
33 } else {
34 x = $[0];
35 }
36 -
36 const z = x.t;
37 return z;
38 }
compiler/forget/src/__tests__/fixtures/compiler/type-test-polymorphic.expect.md
+2 -1
@@ -43,13 +43,14 @@ function component() {
43 let x;
44 if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
45 x = {};
46 +
47 x.t = p;
48 +
49 x.t = o;
50 $[2] = x;
51 } else {
52 x = $[2];
53 }
52 -
54 const y = x.t;
55 return y;
56 }