From dc39273a8e060bb81cae20517544fa694f8c0d1d Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 18 Jul 2024 15:32:54 -0400 Subject: [PATCH] [compiler][patch] Don't wrap non-ascii jsx operands in JSXExpressionContainer [ghstack-poisoned] --- .../src/Entrypoint/Pipeline.ts | 7 +- .../ReactiveScopes/CodegenReactiveFunction.ts | 22 +++++- .../MemoizeFbtAndMacroOperandsInSameScope.ts | 5 +- ...rror.todo-fbt-param-with-newline.expect.md | 36 --------- ...error.todo-fbt-param-with-quotes.expect.md | 30 -------- ...rror.todo-fbt-param-with-unicode.expect.md | 30 -------- .../fbt/fbt-param-with-newline.expect.md | 75 +++++++++++++++++++ ...h-newline.js => fbt-param-with-newline.js} | 0 .../fbt/fbt-param-with-quotes.expect.md | 61 +++++++++++++++ ...ith-quotes.js => fbt-param-with-quotes.js} | 0 .../fbt/fbt-param-with-unicode.expect.md | 63 ++++++++++++++++ ...h-unicode.js => fbt-param-with-unicode.js} | 0 12 files changed, 227 insertions(+), 102 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-newline.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-quotes.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-unicode.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-newline.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/{error.todo-fbt-param-with-newline.js => fbt-param-with-newline.js} (100%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-quotes.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/{error.todo-fbt-param-with-quotes.js => fbt-param-with-quotes.js} (100%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-unicode.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/{error.todo-fbt-param-with-unicode.js => fbt-param-with-unicode.js} (100%) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index b1480d76dc0f3..077d30f121a30 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -262,7 +262,7 @@ function* runWithEnvironment( value: hir, }); - memoizeFbtOperandsInSameScope(hir); + const fbtOperands = memoizeFbtOperandsInSameScope(hir); yield log({ kind: "hir", name: "MemoizeFbtAndMacroOperandsInSameScope", @@ -484,7 +484,10 @@ function* runWithEnvironment( validatePreservedManualMemoization(reactiveFunction); } - const ast = codegenFunction(reactiveFunction, uniqueIdentifiers).unwrap(); + const ast = codegenFunction(reactiveFunction, { + uniqueIdentifiers, + fbtOperands, + }).unwrap(); yield log({ kind: "ast", name: "Codegen", value: ast }); for (const outlined of ast.outlined) { yield log({ kind: "ast", name: "Codegen (outlined)", value: outlined.fn }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index ff11c09939aca..bd44b18423536 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -100,12 +100,19 @@ export type CodegenFunction = { export function codegenFunction( fn: ReactiveFunction, - uniqueIdentifiers: Set + { + uniqueIdentifiers, + fbtOperands, + }: { + uniqueIdentifiers: Set; + fbtOperands: Set; + } ): Result { const cx = new Context( fn.env, fn.id ?? "[[ anonymous ]]", uniqueIdentifiers, + fbtOperands, null ); @@ -281,7 +288,8 @@ export function codegenFunction( new Context( cx.env, reactiveFunction.id ?? "[[ anonymous ]]", - identifiers + identifiers, + cx.fbtOperands ), reactiveFunction ); @@ -391,17 +399,20 @@ class Context { errors: CompilerError = new CompilerError(); objectMethods: Map = new Map(); uniqueIdentifiers: Set; + fbtOperands: Set; synthesizedNames: Map = new Map(); constructor( env: Environment, fnName: string, uniqueIdentifiers: Set, + fbtOperands: Set, temporaries: Temporaries | null = null ) { this.env = env; this.fnName = fnName; this.uniqueIdentifiers = uniqueIdentifiers; + this.fbtOperands = fbtOperands; this.temp = temporaries !== null ? new Map(temporaries) : new Map(); } get nextCacheIndex(): number { @@ -1776,6 +1787,7 @@ function codegenInstructionValue( cx.env, reactiveFunction.id ?? "[[ anonymous ]]", cx.uniqueIdentifiers, + cx.fbtOperands, cx.temp ), reactiveFunction @@ -1979,6 +1991,7 @@ function codegenInstructionValue( cx.env, reactiveFunction.id ?? "[[ anonymous ]]", cx.uniqueIdentifiers, + cx.fbtOperands, cx.temp ), reactiveFunction @@ -2229,7 +2242,10 @@ function codegenJsxAttribute( switch (innerValue.type) { case "StringLiteral": { value = innerValue; - if (STRING_REQUIRES_EXPR_CONTAINER_PATTERN.test(value.value)) { + if ( + STRING_REQUIRES_EXPR_CONTAINER_PATTERN.test(value.value) && + !cx.fbtOperands.has(attribute.place.identifier.id) + ) { value = createJsxExpressionContainer(value.loc, value); } break; diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts index 91121cd9bfbac..037765f1e9cab 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts @@ -39,7 +39,9 @@ import { eachReactiveValueOperand } from "./visitors"; * Users can also specify their own functions to be treated similarly to fbt via the * `customMacros` environment configuration. */ -export function memoizeFbtAndMacroOperandsInSameScope(fn: HIRFunction): void { +export function memoizeFbtAndMacroOperandsInSameScope( + fn: HIRFunction +): Set { const fbtMacroTags = new Set([ ...FBT_TAGS, ...(fn.env.config.customMacros ?? []), @@ -52,6 +54,7 @@ export function memoizeFbtAndMacroOperandsInSameScope(fn: HIRFunction): void { break; } } + return fbtValues; } export const FBT_TAGS: Set = new Set([ diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-newline.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-newline.expect.md deleted file mode 100644 index 054804c3e1d29..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-newline.expect.md +++ /dev/null @@ -1,36 +0,0 @@ - -## Input - -```javascript -import fbt from "fbt"; - -function Component(props) { - const element = ( - - Hello{" "} - - {props.name} - - - ); - return element.toString(); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ name: "Jason" }], -}; - -``` - - -## Error - -``` -Cannot read properties of undefined (reading 'replace') -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-quotes.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-quotes.expect.md deleted file mode 100644 index 9d616baf58b08..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-quotes.expect.md +++ /dev/null @@ -1,30 +0,0 @@ - -## Input - -```javascript -import fbt from "fbt"; - -function Component(props) { - const element = ( - - Hello {props.name} - - ); - return element.toString(); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ name: "Jason" }], -}; - -``` - - -## Error - -``` -Property arguments[0] of CallExpression expected node to be of a type ["Expression","SpreadElement","JSXNamespacedName","ArgumentPlaceholder"] but instead got "JSXExpressionContainer" -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-unicode.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-unicode.expect.md deleted file mode 100644 index ef75314048d65..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-unicode.expect.md +++ /dev/null @@ -1,30 +0,0 @@ - -## Input - -```javascript -import fbt from "fbt"; - -function Component(props) { - const element = ( - - Hello {props.name} - - ); - return element.toString(); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ name: "Jason" }], -}; - -``` - - -## Error - -``` -Property arguments[0] of CallExpression expected node to be of a type ["Expression","SpreadElement","JSXNamespacedName","ArgumentPlaceholder"] but instead got "JSXExpressionContainer" -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-newline.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-newline.expect.md new file mode 100644 index 0000000000000..67402ba3bd788 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-newline.expect.md @@ -0,0 +1,75 @@ + +## Input + +```javascript +import fbt from "fbt"; + +function Component(props) { + const element = ( + + Hello{" "} + + {props.name} + + + ); + return element.toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import fbt from "fbt"; + +function Component(props) { + const $ = _c(4); + let t0; + if ($[0] !== props.name) { + t0 = fbt._( + "Hello {a really long description that got split into multiple lines}", + [ + fbt._param( + "a really long description that got split into multiple lines", + + props.name, + ), + ], + { hk: "1euPUp" }, + ); + $[0] = props.name; + $[1] = t0; + } else { + t0 = $[1]; + } + const element = t0; + let t1; + if ($[2] !== element) { + t1 = element.toString(); + $[2] = element; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +### Eval output +(kind: ok) "Hello Jason" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-newline.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-newline.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-newline.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-newline.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-quotes.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-quotes.expect.md new file mode 100644 index 0000000000000..4fc0d198c674f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-quotes.expect.md @@ -0,0 +1,61 @@ + +## Input + +```javascript +import fbt from "fbt"; + +function Component(props) { + const element = ( + + Hello {props.name} + + ); + return element.toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import fbt from "fbt"; + +function Component(props) { + const $ = _c(4); + let t0; + if ($[0] !== props.name) { + t0 = fbt._('Hello {"user" name}', [fbt._param('"user" name', props.name)], { + hk: "S0vMe", + }); + $[0] = props.name; + $[1] = t0; + } else { + t0 = $[1]; + } + const element = t0; + let t1; + if ($[2] !== element) { + t1 = element.toString(); + $[2] = element; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +### Eval output +(kind: ok) "Hello Jason" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-quotes.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-quotes.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-quotes.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-quotes.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-unicode.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-unicode.expect.md new file mode 100644 index 0000000000000..221a585591d74 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-unicode.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +import fbt from "fbt"; + +function Component(props) { + const element = ( + + Hello {props.name} + + ); + return element.toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import fbt from "fbt"; + +function Component(props) { + const $ = _c(4); + let t0; + if ($[0] !== props.name) { + t0 = fbt._( + "Hello {user name ☺}", + [fbt._param("user name \u263A", props.name)], + { hk: "1En1lp" }, + ); + $[0] = props.name; + $[1] = t0; + } else { + t0 = $[1]; + } + const element = t0; + let t1; + if ($[2] !== element) { + t1 = element.toString(); + $[2] = element; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ name: "Jason" }], +}; + +``` + +### Eval output +(kind: ok) "Hello Jason" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-unicode.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-unicode.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-fbt-param-with-unicode.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-param-with-unicode.js