@samitouri / QOS-React-2 / commits / 6f2fe1e7e9

Fix FBT whitespace handling (again (again))

Nice find, @mofeiZ! I really tried to thoroughly test all the examples I could think of for FBT whitespace but i missed the newline case. Initially Mofei found [this code potentially related to whitespace handling](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtImplicitParamNode.js#L230-L233) but Babel never seems to produce consecutive JsxText nodes — this looks like maybe a leftover from older babel versions. I noticed that code wasn't actually trimming the whitspace but clearly it was happening somewhere, so i grepped for 'trim' and found that [this code](https://github.com/facebook/fbt/blob/0b4e0d13c30bffd0daa2a75715d606e3587b4e40/packages/babel-plugin-fbt/src/babel-processors/JSXFbtProcessor.js#L143) calls a [normalizeSpaces](https://github.com/facebook/fbt/blob/0b4e0d13c30bffd0daa2a75715d606e3587b4e40/packages/babel-plugin-fbt/src/FbtUtil.js#L86C10-L86C45) helper. Updating our logic to handle whitespace similarly just for children of fbt nodes produces the expected result.

Joe Savona committed Nov 10, 2023 at 10:21 UTC 6f2fe1e7e97a18992c9aefe86d9f2242535f65cb
5 files changed +35 -20
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+26 -4
@@ -1902,10 +1902,6 @@ function lowerExpression(
1902 const expr = exprPath as NodePath<t.JSXElement>;
1903 const opening = expr.get("openingElement");
1904 const tag = lowerJsxElementName(builder, opening.get("name"));
1905 - const children: Array<Place> = expr
1906 - .get("children")
1907 - .map((child) => lowerJsxElement(builder, child))
1908 - .filter(notNull);
1905 const props: Array<JsxAttribute> = [];
1906 for (const attribute of opening.get("attributes")) {
1907 if (attribute.isJSXSpreadAttribute()) {
@@ -1982,6 +1978,32 @@ function lowerExpression(
1978 }
1979 props.push({ kind: "JsxAttribute", name: propName, place: value });
1980 }
1981 + let children: Array<Place>;
1982 + if (tag.kind === "BuiltinTag" && tag.name === "fbt") {
1983 + children = expr
1984 + .get("children")
1985 + .map((child) => {
1986 + if (child.isJSXText()) {
1987 + /*
1988 + * FBT whitespace normalization differs from standard JSX:
1989 + * https://github.com/facebook/fbt/blob/0b4e0d13c30bffd0daa2a75715d606e3587b4e40/packages/babel-plugin-fbt/src/FbtUtil.js#L76-L87
1990 + */
1991 + const text = child.node.value.replace(/[^\S\u00A0]+/g, " ");
1992 + return lowerValueToTemporary(builder, {
1993 + kind: "JSXText",
1994 + value: text,
1995 + loc: exprLoc,
1996 + });
1997 + }
1998 + return lowerJsxElement(builder, child);
1999 + })
2000 + .filter(notNull);
2001 + } else {
2002 + children = expr
2003 + .get("children")
2004 + .map((child) => lowerJsxElement(builder, child))
2005 + .filter(notNull);
2006 + }
2007 return {
2008 kind: "JsxExpression",
2009 tag,
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbt-preserve-whitespace.expect.md renamed
+6 -10
@@ -3,10 +3,8 @@
3
4 ```javascript
5 import fbt from "fbt";
6 -/**
7 - * TODO: remove this from SproutTodoFilter when fixed.
8 - */
6
7 +const _ = fbt;
8 function Component({ value }: { value: string }) {
9 return (
10 <fbt desc="descdesc">
@@ -28,17 +26,15 @@ export const FIXTURE_ENTRYPOINT = {
26 ```javascript
27 import { unstable_useMemoCache as useMemoCache } from "react";
28 import fbt from "fbt";
31 -/**
32 - * TODO: remove this from SproutTodoFilter when fixed.
33 - */
29
35 -function Component(t11) {
30 +const _ = fbt;
31 +function Component(t12) {
32 const $ = useMemoCache(2);
37 - const { value } = t11;
33 + const { value } = t12;
34 let t0;
35 if ($[0] !== value) {
36 t0 = fbt._(
41 - "Before text{paramName}",
37 + "Before text {paramName}",
38 [
39 fbt._param(
40 "paramName",
@@ -46,7 +42,7 @@ function Component(t11) {
42 value
43 ),
44 ],
49 - { hk: "3DIRxJ" }
45 + { hk: "3z5SVE" }
46 );
47 $[0] = value;
48 $[1] = t0;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbt-preserve-whitespace.tsx renamed
+1 -3
@@ -1,8 +1,6 @@
1 import fbt from "fbt";
2 -/**
3 - * TODO: remove this from SproutTodoFilter when fixed.
4 - */
2
3 +const _ = fbt;
4 function Component({ value }: { value: string }) {
5 return (
6 <fbt desc="descdesc">
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.expect.md
+2 -2
@@ -28,9 +28,9 @@ function Component({ name, data, icon }) {
28 import { unstable_useMemoCache as useMemoCache } from "react";
29 import fbt from "fbt";
30
31 -function Component(t29) {
31 +function Component(t33) {
32 const $ = useMemoCache(4);
33 - const { name, data, icon } = t29;
33 + const { name, data, icon } = t33;
34 let t0;
35 if ($[0] !== name || $[1] !== icon || $[2] !== data) {
36 t0 = (
compiler/packages/sprout/src/SproutTodoFilter.ts
-1
@@ -469,7 +469,6 @@ const skipFilter = new Set([
469
470 // Bug in Forget output
471 "todo-rename-source-variables",
472 - "bug-fbt-preserve-whitespace",
472
473 // Tested e2e in forget-feedback repo
474 "userspace-use-memo-cache",