From 1137e75b44db994439a21594b5d99ef1dd6b0b3d Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Tue, 23 Jul 2024 17:52:22 -0400 Subject: [PATCH] [compiler] Todo for fbt with multiple pronoun/plural [ghstack-poisoned] --- .../src/HIR/BuildHIR.ts | 43 ++++--- .../fbt/bug-multiple-fbt-plural.expect.md | 110 ------------------ .../error.todo-multiple-fbt-plural.expect.md | 65 +++++++++++ ...tsx => error.todo-multiple-fbt-plural.tsx} | 22 ++-- .../fbt/fbt-preserve-jsxtext.expect.md | 17 ++- .../compiler/fbt/fbt-preserve-jsxtext.js | 6 + .../packages/snap/src/SproutTodoFilter.ts | 1 - 7 files changed, 129 insertions(+), 135 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-multiple-fbt-plural.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-multiple-fbt-plural.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/{bug-multiple-fbt-plural.tsx => error.todo-multiple-fbt-plural.tsx} (53%) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts index 9ed3f84ca2a6e..78aaa3e7c3273 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts @@ -2130,24 +2130,41 @@ function lowerExpression( suggestions: null, }); } - const fbtEnumLocations: Array = []; + // see `error.todo-multiple-fbt-plural` fixture for explanation + const fbtLocations = { + enum: new Array(), + plural: new Array(), + pronoun: new Array(), + }; expr.traverse({ + JSXClosingElement(path) { + path.skip(); + }, JSXNamespacedName(path) { - if ( - path.node.namespace.name === tagName && - path.node.name.name === 'enum' - ) { - fbtEnumLocations.push(path.node.loc ?? GeneratedSource); + if (path.node.namespace.name === tagName) { + switch (path.node.name.name) { + case 'enum': + fbtLocations.enum.push(path.node.loc ?? GeneratedSource); + break; + case 'plural': + fbtLocations.plural.push(path.node.loc ?? GeneratedSource); + break; + case 'pronoun': + fbtLocations.pronoun.push(path.node.loc ?? GeneratedSource); + break; + } } }, }); - if (fbtEnumLocations.length > 1) { - CompilerError.throwTodo({ - reason: `Support <${tagName}> tags with multiple <${tagName}:enum> values`, - loc: fbtEnumLocations.at(-1) ?? GeneratedSource, - description: null, - suggestions: null, - }); + for (const [name, locations] of Object.entries(fbtLocations)) { + if (locations.length > 1) { + CompilerError.throwTodo({ + reason: `Support <${tagName}> tags with multiple <${tagName}:${name}> values`, + loc: locations.at(-1) ?? GeneratedSource, + description: null, + suggestions: null, + }); + } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-multiple-fbt-plural.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-multiple-fbt-plural.expect.md deleted file mode 100644 index dd09671a2478d..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-multiple-fbt-plural.expect.md +++ /dev/null @@ -1,110 +0,0 @@ - -## Input - -```javascript -import fbt from 'fbt'; - -/** - * Forget + fbt inconsistency. Evaluator errors with the following - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) 1 rewrite to Rust · 2 months traveling - * Forget: - * (kind: ok) 1 rewrites to Rust · 2 months traveling - * - * The root issue here is that fbt:plural reads `.start` and `.end` from - * babel nodes to slice into source strings. (fbt:enum suffers from the same - * problem). - * See: - * - [getRawSource](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/FbtUtil.js#L666-L673) - * - [getArgCode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtArguments.js#L88-L97) - * - [_getStringVariationCombinations](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/JSFbtBuilder.js#L297) - * - * Specifically, the `count` node requires that a `.start/.end` be attached - * (see [code in FbtPluralNode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtPluralNode.js#L87-L90)) - * - * In this fixture, `count` nodes are the `rewrites` and `months` identifiers. - */ -function Foo({rewrites, months}) { - return ( - - - rewrite - - to Rust · - - month - - traveling - - ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{rewrites: 1, months: 2}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import fbt from "fbt"; - -/** - * Forget + fbt inconsistency. Evaluator errors with the following - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) 1 rewrite to Rust · 2 months traveling - * Forget: - * (kind: ok) 1 rewrites to Rust · 2 months traveling - * - * The root issue here is that fbt:plural reads `.start` and `.end` from - * babel nodes to slice into source strings. (fbt:enum suffers from the same - * problem). - * See: - * - [getRawSource](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/FbtUtil.js#L666-L673) - * - [getArgCode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtArguments.js#L88-L97) - * - [_getStringVariationCombinations](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/JSFbtBuilder.js#L297) - * - * Specifically, the `count` node requires that a `.start/.end` be attached - * (see [code in FbtPluralNode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtPluralNode.js#L87-L90)) - * - * In this fixture, `count` nodes are the `rewrites` and `months` identifiers. - */ -function Foo(t0) { - const $ = _c(3); - const { rewrites, months } = t0; - let t1; - if ($[0] !== rewrites || $[1] !== months) { - t1 = fbt._( - { - "*": { - "*": "{number of rewrites} rewrites to Rust · {number of months} months traveling", - }, - _1: { _1: "1 rewrite to Rust · 1 month traveling" }, - }, - [ - fbt._plural(rewrites, "number of rewrites"), - fbt._plural(months, "number of months"), - ], - { hk: "49MfZA" }, - ); - $[0] = rewrites; - $[1] = months; - $[2] = t1; - } else { - t1 = $[2]; - } - return t1; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{ rewrites: 1, months: 2 }], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-multiple-fbt-plural.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-multiple-fbt-plural.expect.md new file mode 100644 index 0000000000000..4645ea67b7f98 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-multiple-fbt-plural.expect.md @@ -0,0 +1,65 @@ + +## Input + +```javascript +import fbt from 'fbt'; + +/** + * Forget + fbt inconsistency. Evaluator errors with the following + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) 1 rewrite to Rust · 2 months traveling + * Forget: + * (kind: ok) 1 rewrites to Rust · 2 months traveling + * + * The root issue here is that fbt:plural/enum/pronoun read `.start` and `.end` from + * babel nodes to slice into source strings for some complex dedupe logic + * (see [_getStringVariationCombinations](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/JSFbtBuilder.js#L297)) + * + * + * Since Forget does not add `.start` and `.end` for babel nodes it synthesizes, + * [getRawSource](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/FbtUtil.js#L666-L673) + * simply returns the whole source code string. As a result, all fbt nodes dedupe together + * and _getStringVariationCombinations ends up early exiting (before adding valid candidate values). + * + * + * + * For fbt:plural tags specifically, the `count` node require that a `.start/.end` + * (see [code in FbtPluralNode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtPluralNode.js#L87-L90)) + */ +function Foo({rewrites, months}) { + return ( + + + rewrite + + to Rust · + + month + + traveling + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{rewrites: 1, months: 2}], +}; + +``` + + +## Error + +``` + 31 | + 32 | to Rust · +> 33 | + | ^^^^^^^^^^ Todo: Support tags with multiple values (33:33) + 34 | month + 35 | + 36 | traveling +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-multiple-fbt-plural.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-multiple-fbt-plural.tsx similarity index 53% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-multiple-fbt-plural.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-multiple-fbt-plural.tsx index 806938ae183c3..ef86a4f6cb722 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-multiple-fbt-plural.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/error.todo-multiple-fbt-plural.tsx @@ -8,18 +8,20 @@ import fbt from 'fbt'; * Forget: * (kind: ok) 1 rewrites to Rust · 2 months traveling * - * The root issue here is that fbt:plural reads `.start` and `.end` from - * babel nodes to slice into source strings. (fbt:enum suffers from the same - * problem). - * See: - * - [getRawSource](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/FbtUtil.js#L666-L673) - * - [getArgCode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtArguments.js#L88-L97) - * - [_getStringVariationCombinations](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/JSFbtBuilder.js#L297) + * The root issue here is that fbt:plural/enum/pronoun read `.start` and `.end` from + * babel nodes to slice into source strings for some complex dedupe logic + * (see [_getStringVariationCombinations](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/JSFbtBuilder.js#L297)) + * + * + * Since Forget does not add `.start` and `.end` for babel nodes it synthesizes, + * [getRawSource](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/FbtUtil.js#L666-L673) + * simply returns the whole source code string. As a result, all fbt nodes dedupe together + * and _getStringVariationCombinations ends up early exiting (before adding valid candidate values). * - * Specifically, the `count` node requires that a `.start/.end` be attached - * (see [code in FbtPluralNode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtPluralNode.js#L87-L90)) * - * In this fixture, `count` nodes are the `rewrites` and `months` identifiers. + * + * For fbt:plural tags specifically, the `count` node require that a `.start/.end` + * (see [code in FbtPluralNode](https://github.com/facebook/fbt/blob/main/packages/babel-plugin-fbt/src/fbt-nodes/FbtPluralNode.js#L87-L90)) */ function Foo({rewrites, months}) { return ( diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md index 3aecc01be5396..4b39a6d237d74 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.expect.md @@ -17,6 +17,12 @@ function Foo(props) { ); } +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{value: 1}], + sequentialRenders: [{value: 1}, {value: 0}], +}; + ``` ## Code @@ -49,5 +55,14 @@ function Foo(props) { return t0; } +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ value: 1 }], + sequentialRenders: [{ value: 1 }, { value: 0 }], +}; + ``` - \ No newline at end of file + +### Eval output +(kind: ok) hello 1, +goodbye 0, \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.js index 973485d1c7dd7..7080753c4db4b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-preserve-jsxtext.js @@ -12,3 +12,9 @@ function Foo(props) { ); } + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{value: 1}], + sequentialRenders: [{value: 1}, {value: 0}], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index e04dbe37d7e57..866c497f81f3f 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -439,7 +439,6 @@ const skipFilter = new Set([ 'fbt/fbtparam-with-jsx-element-content', 'fbt/fbtparam-text-must-use-expression-container', 'fbt/fbtparam-with-jsx-fragment-value', - 'fbt/fbt-preserve-jsxtext', 'todo.useContext-mutate-context-in-callback', 'loop-unused-let', 'reanimated-no-memo-arg',