@samitouri / QOS-React / commits / e215aa1160

[compiler] Fix FBT whitespace handling again (again (again))

ghstack-source-id: 00a86e41cfb8a6fb56b7fcd811740d1d9b89a611 Pull Request resolved: https://github.com/facebook/react/pull/30451

Mofei Zhang committed Jul 26, 2024 at 17:36 UTC e215aa1160adc51b8f31788872c11468585d5cc8
9 files changed +254 -167
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+28 -34
@@ -2113,10 +2113,10 @@ function lowerExpression(
2113 }
2114 props.push({kind: 'JsxAttribute', name: propName, place: value});
2115 }
2116 - if (
2117 - tag.kind === 'BuiltinTag' &&
2118 - (tag.name === 'fbt' || tag.name === 'fbs')
2119 - ) {
2116 +
2117 + const isFbt =
2118 + tag.kind === 'BuiltinTag' && (tag.name === 'fbt' || tag.name === 'fbs');
2119 + if (isFbt) {
2120 const tagName = tag.name;
2121 const openingIdentifier = opening.get('name');
2122 const tagIdentifier = openingIdentifier.isJSXIdentifier()
@@ -2168,35 +2168,17 @@ function lowerExpression(
2168 }
2169 }
2170
2171 - let children: Array<Place>;
2172 - if (
2173 - tag.kind === 'BuiltinTag' &&
2174 - (tag.name === 'fbt' || tag.name === 'fbs')
2175 - ) {
2176 - children = expr
2177 - .get('children')
2178 - .map(child => {
2179 - if (child.isJSXText()) {
2180 - /*
2181 - * FBT whitespace normalization differs from standard JSX:
2182 - * https://github.com/facebook/fbt/blob/0b4e0d13c30bffd0daa2a75715d606e3587b4e40/packages/babel-plugin-fbt/src/FbtUtil.js#L76-L87
2183 - */
2184 - const text = child.node.value.replace(/[^\S\u00A0]+/g, ' ');
2185 - return lowerValueToTemporary(builder, {
2186 - kind: 'JSXText',
2187 - value: text,
2188 - loc: child.node.loc ?? GeneratedSource,
2189 - });
2190 - }
2191 - return lowerJsxElement(builder, child);
2192 - })
2193 - .filter(notNull);
2194 - } else {
2195 - children = expr
2196 - .get('children')
2197 - .map(child => lowerJsxElement(builder, child))
2198 - .filter(notNull);
2199 - }
2171 + /**
2172 + * Increment fbt counter before traversing into children, as whitespace
2173 + * in jsx text is handled differently for fbt subtrees.
2174 + */
2175 + isFbt && builder.fbtDepth++;
2176 + const children: Array<Place> = expr
2177 + .get('children')
2178 + .map(child => lowerJsxElement(builder, child))
2179 + .filter(notNull);
2180 + isFbt && builder.fbtDepth--;
2181 +
2182 return {
2183 kind: 'JsxExpression',
2184 tag,
@@ -3158,7 +3140,19 @@ function lowerJsxElement(
3140 return lowerExpressionToTemporary(builder, expression);
3141 }
3142 } else if (exprPath.isJSXText()) {
3161 - const text = trimJsxText(exprPath.node.value);
3143 + let text: string | null;
3144 + if (builder.fbtDepth > 0) {
3145 + /*
3146 + * FBT whitespace normalization differs from standard JSX.
3147 + * https://github.com/facebook/fbt/blob/0b4e0d13c30bffd0daa2a75715d606e3587b4e40/packages/babel-plugin-fbt/src/FbtUtil.js#L76-L87
3148 + * Since the fbt transform runs after, let's just preserve all
3149 + * whitespace in FBT subtrees as is.
3150 + */
3151 + text = exprPath.node.value;
3152 + } else {
3153 + text = trimJsxText(exprPath.node.value);
3154 + }
3155 +
3156 if (text === null) {
3157 return null;
3158 }
compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts
+5
@@ -110,6 +110,11 @@ export default class HIRBuilder {
110 #exceptionHandlerStack: Array<BlockId> = [];
111 parentFunction: NodePath<t.Function>;
112 errors: CompilerError = new CompilerError();
113 + /**
114 + * Traversal context: counts the number of `fbt` tag parents
115 + * of the current babel node.
116 + */
117 + fbtDepth: number = 0;
118
119 get nextIdentifierId(): IdentifierId {
120 return this.#env.nextIdentifierId;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-fbt-preserve-whitespace-param.expect.md deleted
-100
@@ -1,100 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -import fbt from 'fbt';
6 -
7 -/**
8 - * Currently fails with the following:
9 - * Found differences in evaluator results
10 - * Non-forget (expected):
11 - * (kind: ok) <div><span>Jason !</span></div>
12 - * Forget:
13 - * (kind: ok) <div><span>Jason!</span></div>
14 -
15 - */
16 -
17 -function Foo(props) {
18 - return (
19 - // prettier-ignore
20 - <div>
21 - <fbt desc={"Dialog to show to user"}>
22 - <span>
23 - <fbt:param name="user name">
24 - {props.name}
25 - </fbt:param>
26 - !
27 - </span>
28 - </fbt>
29 - </div>
30 - );
31 -}
32 -
33 -export const FIXTURE_ENTRYPOINT = {
34 - fn: Foo,
35 - params: [{name: 'Jason'}],
36 -};
37 -
38 -```
39 -
40 -## Code
41 -
42 -```javascript
43 -import { c as _c } from "react/compiler-runtime";
44 -import fbt from "fbt";
45 -
46 -/**
47 - * Currently fails with the following:
48 - * Found differences in evaluator results
49 - * Non-forget (expected):
50 - * (kind: ok) <div><span>Jason !</span></div>
51 - * Forget:
52 - * (kind: ok) <div><span>Jason!</span></div>
53 -
54 - */
55 -
56 -function Foo(props) {
57 - const $ = _c(2);
58 - let t0;
59 - if ($[0] !== props.name) {
60 - t0 = (
61 - <div>
62 - {fbt._(
63 - "{=m0}",
64 - [
65 - fbt._implicitParam(
66 - "=m0",
67 - <span>
68 - {fbt._(
69 - "{user name}!",
70 - [
71 - fbt._param(
72 - "user name",
73 -
74 - props.name,
75 - ),
76 - ],
77 - { hk: "mBBZ9" },
78 - )}
79 - </span>,
80 - ),
81 - ],
82 - { hk: "3RVfuk" },
83 - )}
84 - </div>
85 - );
86 - $[0] = props.name;
87 - $[1] = t0;
88 - } else {
89 - t0 = $[1];
90 - }
91 - return t0;
92 -}
93 -
94 -export const FIXTURE_ENTRYPOINT = {
95 - fn: Foo,
96 - params: [{ name: "Jason" }],
97 -};
98 -
99 -```
100 -
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-fbt-preserve-whitespace-param.tsx deleted
-32
@@ -1,32 +0,0 @@
1 -import fbt from 'fbt';
2 -
3 -/**
4 - * Currently fails with the following:
5 - * Found differences in evaluator results
6 - * Non-forget (expected):
7 - * (kind: ok) <div><span>Jason !</span></div>
8 - * Forget:
9 - * (kind: ok) <div><span>Jason!</span></div>
10 -
11 - */
12 -
13 -function Foo(props) {
14 - return (
15 - // prettier-ignore
16 - <div>
17 - <fbt desc={"Dialog to show to user"}>
18 - <span>
19 - <fbt:param name="user name">
20 - {props.name}
21 - </fbt:param>
22 - !
23 - </span>
24 - </fbt>
25 - </div>
26 - );
27 -}
28 -
29 -export const FIXTURE_ENTRYPOINT = {
30 - fn: Foo,
31 - params: [{name: 'Jason'}],
32 -};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-subtree.expect.md new
+89
@@ -0,0 +1,89 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import fbt from 'fbt';
6 +
7 +/**
8 + * Note that fbt whitespace rules apply to the entire fbt subtree,
9 + * not just direct children of fbt elements.
10 + * (e.g. here, the JSXText children of the span element also use
11 + * fbt whitespace rules)
12 + */
13 +
14 +function Foo(props) {
15 + return (
16 + <fbt desc={'Dialog to show to user'}>
17 + <span>
18 + <fbt:param name="user name really long description for prettier">
19 + {props.name}
20 + </fbt:param>
21 + !
22 + </span>
23 + </fbt>
24 + );
25 +}
26 +
27 +export const FIXTURE_ENTRYPOINT = {
28 + fn: Foo,
29 + params: [{name: 'Jason'}],
30 +};
31 +
32 +```
33 +
34 +## Code
35 +
36 +```javascript
37 +import { c as _c } from "react/compiler-runtime";
38 +import fbt from "fbt";
39 +
40 +/**
41 + * Note that fbt whitespace rules apply to the entire fbt subtree,
42 + * not just direct children of fbt elements.
43 + * (e.g. here, the JSXText children of the span element also use
44 + * fbt whitespace rules)
45 + */
46 +
47 +function Foo(props) {
48 + const $ = _c(2);
49 + let t0;
50 + if ($[0] !== props.name) {
51 + t0 = fbt._(
52 + "{=m0}",
53 + [
54 + fbt._implicitParam(
55 + "=m0",
56 + <span>
57 + {fbt._(
58 + "{user name really long description for prettier} !",
59 + [
60 + fbt._param(
61 + "user name really long description for prettier",
62 +
63 + props.name,
64 + ),
65 + ],
66 + { hk: "rdgIJ" },
67 + )}
68 + </span>,
69 + ),
70 + ],
71 + { hk: "32Ufy5" },
72 + );
73 + $[0] = props.name;
74 + $[1] = t0;
75 + } else {
76 + t0 = $[1];
77 + }
78 + return t0;
79 +}
80 +
81 +export const FIXTURE_ENTRYPOINT = {
82 + fn: Foo,
83 + params: [{ name: "Jason" }],
84 +};
85 +
86 +```
87 +
88 +### Eval output
89 +(kind: ok) <span>Jason !</span>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-subtree.tsx new
+26
@@ -0,0 +1,26 @@
1 +import fbt from 'fbt';
2 +
3 +/**
4 + * Note that fbt whitespace rules apply to the entire fbt subtree,
5 + * not just direct children of fbt elements.
6 + * (e.g. here, the JSXText children of the span element also use
7 + * fbt whitespace rules)
8 + */
9 +
10 +function Foo(props) {
11 + return (
12 + <fbt desc={'Dialog to show to user'}>
13 + <span>
14 + <fbt:param name="user name really long description for prettier">
15 + {props.name}
16 + </fbt:param>
17 + !
18 + </span>
19 + </fbt>
20 + );
21 +}
22 +
23 +export const FIXTURE_ENTRYPOINT = {
24 + fn: Foo,
25 + params: [{name: 'Jason'}],
26 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-two-subtrees.expect.md new
+81
@@ -0,0 +1,81 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import fbt from 'fbt';
6 +
7 +function Foo({name1, name2}) {
8 + return (
9 + <fbt desc="Text that is displayed when two people accepts the user's pull request.">
10 + <fbt:param name="user1">
11 + <span>
12 + <b>{name1}</b>
13 + </span>
14 + </fbt:param>
15 + and
16 + <fbt:param name="user2">
17 + <span>
18 + <b>{name2}</b>
19 + </span>
20 + </fbt:param>
21 + accepted your PR!
22 + </fbt>
23 + );
24 +}
25 +
26 +export const FIXTURE_ENTRYPOINT = {
27 + fn: Foo,
28 + params: [{name1: 'Mike', name2: 'Jan'}],
29 +};
30 +
31 +```
32 +
33 +## Code
34 +
35 +```javascript
36 +import { c as _c } from "react/compiler-runtime";
37 +import fbt from "fbt";
38 +
39 +function Foo(t0) {
40 + const $ = _c(3);
41 + const { name1, name2 } = t0;
42 + let t1;
43 + if ($[0] !== name1 || $[1] !== name2) {
44 + t1 = fbt._(
45 + "{user1} and {user2} accepted your PR!",
46 + [
47 + fbt._param(
48 + "user1",
49 +
50 + <span>
51 + <b>{name1}</b>
52 + </span>,
53 + ),
54 + fbt._param(
55 + "user2",
56 +
57 + <span>
58 + <b>{name2}</b>
59 + </span>,
60 + ),
61 + ],
62 + { hk: "2PxMie" },
63 + );
64 + $[0] = name1;
65 + $[1] = name2;
66 + $[2] = t1;
67 + } else {
68 + t1 = $[2];
69 + }
70 + return t1;
71 +}
72 +
73 +export const FIXTURE_ENTRYPOINT = {
74 + fn: Foo,
75 + params: [{ name1: "Mike", name2: "Jan" }],
76 +};
77 +
78 +```
79 +
80 +### Eval output
81 +(kind: ok) <span><b>Mike</b></span> and <span><b>Jan</b></span> accepted your PR!
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-whitespace-two-subtrees.tsx new
+25
@@ -0,0 +1,25 @@
1 +import fbt from 'fbt';
2 +
3 +function Foo({name1, name2}) {
4 + return (
5 + <fbt desc="Text that is displayed when two people accepts the user's pull request.">
6 + <fbt:param name="user1">
7 + <span>
8 + <b>{name1}</b>
9 + </span>
10 + </fbt:param>
11 + and
12 + <fbt:param name="user2">
13 + <span>
14 + <b>{name2}</b>
15 + </span>
16 + </fbt:param>
17 + accepted your PR!
18 + </fbt>
19 + );
20 +}
21 +
22 +export const FIXTURE_ENTRYPOINT = {
23 + fn: Foo,
24 + params: [{name1: 'Mike', name2: 'Jan'}],
25 +};
compiler/packages/snap/src/SproutTodoFilter.ts
-1
@@ -484,7 +484,6 @@ const skipFilter = new Set([
484 'rules-of-hooks/rules-of-hooks-69521d94fa03',
485
486 // bugs
487 - 'fbt/bug-fbt-preserve-whitespace-param',
487 'bug-invalid-hoisting-functionexpr',
488 'original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block',
489 'original-reactive-scopes-fork/bug-hoisted-declaration-with-scope',