Skip to content

Commit

Permalink
[compiler] Todo for fbt with multiple pronoun/plural
Browse files Browse the repository at this point in the history
ghstack-source-id: 2be0fa3784b9a7623118520deef07d669d975665
Pull Request resolved: #30437
  • Loading branch information
mofeiZ committed Jul 23, 2024
1 parent 8f8674b commit d195811
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 135 deletions.
43 changes: 30 additions & 13 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2130,24 +2130,41 @@ function lowerExpression(
suggestions: null,
});
}
const fbtEnumLocations: Array<SourceLocation> = [];
// see `error.todo-multiple-fbt-plural` fixture for explanation
const fbtLocations = {
enum: new Array<SourceLocation>(),
plural: new Array<SourceLocation>(),
pronoun: new Array<SourceLocation>(),
};
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,
});
}
}
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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 (
<fbt desc="Test fbt description">
<fbt:plural count={rewrites} name="number of rewrites" showCount="yes">
rewrite
</fbt:plural>
to Rust ·
<fbt:plural count={months} name="number of months" showCount="yes">
month
</fbt:plural>
traveling
</fbt>
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Foo,
params: [{rewrites: 1, months: 2}],
};

```


## Error

```
31 | </fbt:plural>
32 | to Rust ·
> 33 | <fbt:plural count={months} name="number of months" showCount="yes">
| ^^^^^^^^^^ Todo: Support <fbt> tags with multiple <fbt:plural> values (33:33)
34 | month
35 | </fbt:plural>
36 | traveling
```
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ function Foo(props) {
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Foo,
params: [{value: 1}],
sequentialRenders: [{value: 1}, {value: 0}],
};

```

## Code
Expand Down Expand Up @@ -49,5 +55,14 @@ function Foo(props) {
return t0;
}

export const FIXTURE_ENTRYPOINT = {
fn: Foo,
params: [{ value: 1 }],
sequentialRenders: [{ value: 1 }, { value: 0 }],
};

```
### Eval output
(kind: ok) hello 1,
goodbye 0,
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,9 @@ function Foo(props) {
</fbt>
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Foo,
params: [{value: 1}],
sequentialRenders: [{value: 1}, {value: 0}],
};
1 change: 0 additions & 1 deletion compiler/packages/snap/src/SproutTodoFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit d195811

Please sign in to comment.