From dc4e63e2d53f91239fe85ac657666c95fde01578 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 15 Sep 2023 11:30:01 -0700 Subject: [PATCH] fbt:param does not allow jsxtext children Per the title, `0` is invalid FBT, you must wrap the text in an expression container. But that's not all, `fbt:param` can only have a single child, which means we have to strip out the text elements that occur from the whitespace in the source. --- .../ReactiveScopes/CodegenReactiveFunction.ts | 57 +++++++++++++++++-- .../MemoizeFbtOperandsInSameScope.ts | 3 +- ...xt-must-use-expression-container.expect.md | 47 +++++++++++++++ ...aram-text-must-use-expression-container.js | 13 +++++ ...btparam-with-jsx-element-content.expect.md | 3 +- .../fbtparam-with-jsx-element-content.js | 1 - .../packages/sprout/src/SproutTodoFilter.ts | 1 + 7 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.js diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 0a7b64d943..ecfb030ab9 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -34,6 +34,7 @@ import { eachPatternOperand } from "../HIR/visitors"; import { Err, Ok, Result } from "../Utils/Result"; import { assertExhaustive } from "../Utils/utils"; import { buildReactiveFunction } from "./BuildReactiveFunction"; +import { SINGLE_CHILD_FBT_TAGS } from "./MemoizeFbtOperandsInSameScope"; export type CodegenFunction = { type: "CodegenFunction"; @@ -990,10 +991,34 @@ function codegenInstructionValue( tag = createJsxIdentifier(instrValue.loc, tagValue.value); } } - const children = - instrValue.children !== null - ? instrValue.children.map((child) => codegenJsxElement(cx, child)) - : []; + let children; + if ( + tagValue.type === "StringLiteral" && + SINGLE_CHILD_FBT_TAGS.has(tagValue.value) + ) { + CompilerError.invariant( + instrValue.children != null && + (instrValue.children.length === 3 || + instrValue.children.length === 1), + { + loc: instrValue.loc, + reason: + "Expected fbt element to have 3 children (whitespace, content, whitespace) or 1 (content)", + suggestions: null, + description: null, + } + ); + if (instrValue.children.length === 3) { + children = [codegenJsxFbtChildElement(cx, instrValue.children[1]!)]; + } else { + children = [codegenJsxFbtChildElement(cx, instrValue.children[0]!)]; + } + } else { + children = + instrValue.children !== null + ? instrValue.children.map((child) => codegenJsxElement(cx, child)) + : []; + } value = createJsxElement( instrValue.loc, t.jsxOpeningElement(tag, attributes, instrValue.children === null), @@ -1330,6 +1355,30 @@ function codegenJsxElement( } } +function codegenJsxFbtChildElement( + cx: Context, + place: Place +): + | t.JSXText + | t.JSXExpressionContainer + | t.JSXSpreadChild + | t.JSXElement + | t.JSXFragment { + const value = codegenPlace(cx, place); + switch (value.type) { + case "StringLiteral": { + return createJsxExpressionContainer(place.loc, value); + } + case "JSXElement": + case "JSXFragment": { + return value; + } + default: { + return createJsxExpressionContainer(place.loc, value); + } + } +} + function convertMemberExpressionToJsx( expr: t.MemberExpression ): t.JSXMemberExpression { diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts index 8673d2a757..d43442066b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts @@ -44,7 +44,8 @@ export function memoizeFbtOperandsInSameScope(fn: ReactiveFunction): void { } } -const FBT_TAGS: Set = new Set(["fbt", "fbt:param"]); +export const FBT_TAGS: Set = new Set(["fbt", "fbt:param"]); +export const SINGLE_CHILD_FBT_TAGS: Set = new Set(["fbt:param"]); class Transform extends ReactiveFunctionVisitor { // Values that represent *potential* references of `fbt` as a JSX tag name diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.expect.md new file mode 100644 index 0000000000..c8de7c1070 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.expect.md @@ -0,0 +1,47 @@ + +## Input + +```javascript +import fbt from "fbt"; + +function Component(props) { + return ( + + {"0"}% + + } + /> + ); +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +import fbt from "fbt"; + +function Component(props) { + const $ = useMemoCache(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = fbt._("{value}%", [fbt._param("value", "0")], { hk: "10F5Cc" }); + $[0] = t0; + } else { + t0 = $[0]; + } + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = ; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.js new file mode 100644 index 0000000000..df9c5d8ad4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-text-must-use-expression-container.js @@ -0,0 +1,13 @@ +import fbt from "fbt"; + +function Component(props) { + return ( + + {"0"}% + + } + /> + ); +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.expect.md index 886cd3de01..c73d301680 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.expect.md @@ -2,7 +2,6 @@ ## Input ```javascript -// @debug import fbt from "fbt"; function Component({ name, data, icon }) { @@ -26,7 +25,7 @@ function Component({ name, data, icon }) { ## Code ```javascript -import { unstable_useMemoCache as useMemoCache } from "react"; // @debug +import { unstable_useMemoCache as useMemoCache } from "react"; import fbt from "fbt"; function Component(t39) { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.js index eed8c729d6..878e2cf4aa 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbtparam-with-jsx-element-content.js @@ -1,4 +1,3 @@ -// @debug import fbt from "fbt"; function Component({ name, data, icon }) { diff --git a/compiler/packages/sprout/src/SproutTodoFilter.ts b/compiler/packages/sprout/src/SproutTodoFilter.ts index 9f2ddb2cab..fbae8134c3 100644 --- a/compiler/packages/sprout/src/SproutTodoFilter.ts +++ b/compiler/packages/sprout/src/SproutTodoFilter.ts @@ -455,6 +455,7 @@ const skipFilter = new Set([ "infer-skip-components-without-hooks-or-jsx", "class-component-with-render-helper", "fbtparam-with-jsx-element-content", + "fbtparam-text-must-use-expression-container", ]); export default skipFilter;