Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3b185e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
examples
|
Warning Review limit reached
Next review available in: 39 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
WalkthroughThe PR aligns JSX message extraction with React whitespace semantics. It merges static expression text, escapes ICU literals, preserves edge whitespace, updates examples and documentation, and adds parser, conversion, extraction, and runtime coverage. ChangesJSX text handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant JSXMessage
participant parseJSXContainerElement
participant convert
participant RuntimeFormatter
JSXMessage->>parseJSXContainerElement: provide JSX children
parseJSXContainerElement-->>convert: return literal and dynamic message parts
convert-->>RuntimeFormatter: provide ICU-escaped message
RuntimeFormatter-->>JSXMessage: render formatted text
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR aligns extracted JSX whitespace with Babel’s JSX child normalization and escapes literal ICU braces while preserving deliberate edge whitespace.
Confidence Score: 5/5The PR appears safe to merge with no concrete changed-code defects identified. The production changes are consistent with the documented JSX and ICU contracts, and the updated unit and integration coverage exercises the principal whitespace, literal-expression, brace, apostrophe, and edge-whitespace paths. Reviews (1): Last reviewed commit: "Extract JSX whitespace as JSX renders it..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/config/src/features/messages/convert.ts`:
- Around line 24-40: Update escapeIcuLiteral to double apostrophes whenever they
precede generated ICU syntax, including opening braces and # tokens, so
separately escaped literal segments cannot quote placeholders or plural/ordinal
replacements. Preserve the existing brace-run escaping and ensure apostrophes
before # are escaped before ICU generation.
In `@packages/transform-jsx/src/parser.ts`:
- Around line 40-51: Update getExpressionAsLiteralText to return an empty string
for Boolean and null literals, and recognize unary plus/minus numeric literals
before dynamic expression handling. Ensure the children reduction in
parseJSXElement extracts <Say>{false}{null}{-1}</Say> as a single argument with
identifier 0 and value -1, and add the corresponding regression test.
In `@website/content/core-concepts/messages.mdx`:
- Around line 435-437: Update the brace explanation near the ICU syntax example
to state that bare braces are invalid in JSX text, then document both JSX
expressions and the character entities &`#123`; and &`#125`; as valid ways to
provide literal braces, with extraction escaping either form for ICU.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3a548c8-a123-4b6b-8c81-c5be6bfdf369
📒 Files selected for processing (14)
.changeset/light-donkeys-hear.md.changeset/olive-pugs-smile.mdexamples/expo/src/habit-card.tsxexamples/react/src/components/board.tsxexamples/tanstack-start/src/routes/{-$locale}/index.tsxpackages/config/src/features/messages/convert.test.tspackages/config/src/features/messages/convert.tspackages/integration/src/runtime.test.tspackages/transform-jsx/src/index.test.tspackages/transform-jsx/src/parser.test.tspackages/transform-jsx/src/parser.tspackages/transform-jsx/src/whitespace.test.tswebsite/content/core-concepts/messages.mdxwebsite/content/integrations/react.mdx
| const children = t.react.buildChildren(element).reduce<Message[]>((p, c) => { | ||
| const literal = getExpressionAsLiteralText(c); | ||
|
|
||
| if (literal !== undefined) { | ||
| pushLiteral(p, literal); | ||
| } else if (t.isJSXElement(c)) { | ||
| p.push(parseJSXElement(c, true)); | ||
| } else if (t.isJSXFragment(c)) { | ||
| p.push(new ElementMessage(AUTO_INCREMENT_IDENTIFIER, [], c)); | ||
| } else if (t.isJSXExpressionContainer(c)) { | ||
| if (t.isExpression(c.expression)) { | ||
| const [identifier, value] = unwrapPlaceholder(c.expression); | ||
| p.push(new ArgumentMessage(identifier, value)); | ||
| } | ||
| } else if (t.isExpression(c)) { | ||
| const [identifier, value] = unwrapPlaceholder(c); | ||
| p.push(new ArgumentMessage(identifier, value)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For @babel/types 7.29.7, verify t.react.buildChildren output for <Say>{false}{null}{-1}</Say>, and verify the corresponding React 19 rendered children.
💡 Result:
For @babel/types 7.29.7, the function t.react.buildChildren(node) processes JSX children by iterating through the child nodes of a JSX element or fragment [1]. It specifically handles JSXExpressionContainer nodes by unwrapping them to their expression and discards JSXEmptyExpression nodes [1]. For the input {false}{null}{-1}: 1. The JSX children are represented as three JSXExpressionContainer nodes: {false}, {null}, and {-1}. 2. t.react.buildChildren extracts the expression from each container, resulting in a list containing the identifiers/literals: false, null, and -1. 3. Because these are not JSXEmptyExpression nodes, they are included in the output array [1]. In React 19, the rendering behavior for these children is as follows: - {false}: Booleans (true/false) are ignored by React and do not render anything in the DOM [2][3]. - {null}: null values are ignored by React and do not render anything [2][3]. - {-1}: Numbers are rendered as their string representation; thus, -1 will be rendered in the DOM as the text "-1". In summary, while the Babel utility retains these as expressions in the AST, React's runtime reconciliation ignores the boolean and null values while rendering the number -1 as visible text.
Citations:
- 1: https://git.ufz.de/khant/metalabel/-/blob/051605d988e8cd29f21b57200563bae1e1659f9a/docs/node_modules/@babel/types/lib/builders/react/buildChildren.js.map
- 2: https://github.com/facebook/react/blob/fac78affb0dd64537e6154cb36bc6f50f57eb049/CHANGELOG.md
- 3: https://it.react.dev/blog/2024/12/05/react-19
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)parser\.ts$|(^|/)package\.json$|(^|/)pnpm-lock\.yaml$|(^|/)yarn\.lock$|(^|/)package-lock\.json$' || true
echo "== target file outline =="
ast-grep outline packages/transform-jsx/src/parser.ts --view compact || true
echo "== relevant parser sections =="
sed -n '1,120p' packages/transform-jsx/src/parser.ts
sed -n '180,235p' packages/transform-jsx/src/parser.ts
echo "== usages of getExpressionAsLiteralText and static child handling =="
rg -n "getExpressionAsLiteralText|isBooleanLiteral|nullLiteral|NumericLiteral|UnaryExpression|extract|Message" packages/transform-jsx/src packages/transform-jsx/test 2>/dev/null || true
echo "== declarations and exports =="
rg -n "class (ElementMessage|ArgumentMessage|.*Message)|interface .*Message|message|placeholders|catalogue|interpolat|extract" packages/transform-jsx/src packages/transform-jsx -g '!**/node_modules/**' 2>/dev/null || trueRepository: k0d13/saykit
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ArgumentMessage signature/deps =="
find . -path '*node_modules' -prune -o -type f -print | rg 'messages\.(ts|tsx|js|jsx)$|features/messages' | xargs -r sed -n '1,220p'
echo "== parse static child tests =="
sed -n '380,480p' packages/transform-jsx/src/parser.test.ts
echo "== deterministic AST probe (repo dependency if present) =="
node - <<'JS'
try {
const babel = require('`@babel/types`');
const { parse } = require('`@babel/parser`');
const code = '<Say>{false}{null}{-1}</Say>';
const ast = parse(code, { sourceType: 'unambiguous', plugins: ['jsx'] }).program.body[0];
const containerEl = ast.expression;
console.log(JSON.stringify(
babel.types.map(babel.types.react.buildChildren(containerEl), c => ({
type: c.type,
exprType: c.type === 'JSXEmptyExpression' ? undefined : c.type,
value: c.type === 'BooleanLiteral' || c.type === 'NullLiteral' || c.type === 'NumericLiteral' ? c.value :
c.type === 'UnaryExpression' ? { operator: c.operator, argumentType: c.argument.type, argumentValue: c.argument.value } : undefined,
})),
null,
2
));
} catch (e) {
console.log(e && e.message);
}
JS
echo "== extraction model probe =="
node - <<'JS'
function buildChildrenChildren(nodes) {
const out = [];
for (const child of nodes) {
if (child.type === 'JSXExpressionContainer') {
const expr = child.expression;
if (expr.type === 'JSXEmptyExpression') continue;
out.push(expr);
} else {
out.push(child);
}
}
return out;
}
function getExpressionAsLiteralText(node) {
if (node.type === 'StringLiteral') return node.value;
if (node.type === 'NumericLiteral') return String(node.value);
return undefined;
}
const input = [
{ type: 'JSXExpressionContainer', expression: { type: 'BooleanLiteral', value: false } },
{ type: 'JSXExpressionContainer', expression: { type: 'NullLiteral' } },
{ type: 'JSXExpressionContainer', expression: { type: 'UnaryExpression', operator: '-', argument: { type: 'NumericLiteral', value: 1 } } },
];
for (const child of buildChildrenChildren(input)) {
const literal = getExpressionAsLiteralText(child);
console.log(child.type, literal ?? 'argument:' + JSON.stringify(child));
}
JSRepository: k0d13/saykit
Length of output: 13147
🌐 Web query:
React 19 DOM rendering Boolean children null children JSX expression container number negative number documentation
💡 Result:
In React 19, the behavior for rendering children in JSX remains consistent with previous versions regarding null, booleans, and numbers [1][2][3]. Children defined as null, undefined, or boolean values (true or false) are valid React nodes but are ignored by the renderer and do not produce any output in the DOM [1][2][3]. This is frequently used for conditional rendering patterns, such as using logical AND (&&) expressions [1][4]. Regarding numbers: - Numbers are treated as renderable content and will be rendered to the DOM as strings [1][3][5]. - A common pitfall occurs when using the logical AND operator (&&) with numbers [1][4]. Because 0 is a falsy value in JavaScript, an expression like {count && } will render 0 to the DOM if count is 0, rather than rendering nothing [1][4]. - To avoid rendering 0, you must ensure the left-hand side of the expression evaluates to a boolean (e.g., {count > 0 && } or {!!count && }) [1][4]. - Negative numbers are treated identically to positive numbers and will be rendered as their string representation [4]. Technical updates in React 19 relevant to types and JSX include: - The removal of several deprecated TypeScript types, such as ReactChild, ReactText, and ReactFragment, in favor of updated definitions (e.g., ReactNode, or explicit union types like string | number) [6]. - The transition of the JSX namespace to be imported from the react package (import { JSX } from 'react') to avoid global scope pollution [7][6]. - While the internal handling of these children remains stable, you should continue to rely on ReactNode for typing children props, as it explicitly includes boolean, null, undefined, string, and number [3][5].
Citations:
- 1: https://legacy.reactjs.org/docs/jsx-in-depth.html
- 2: https://react.dev/reference/react/Children
- 3: https://blog.logrocket.com/react-children-prop-typescript/
- 4: https://stackoverflow.com/questions/37566894/jsx-doesnt-evaluate-integer-in-expression-as-boolean
- 5: https://www.arahansen.com/how-children-types-work-in-react-18-and-typescript-4/
- 6: https://github.com/react/react/releases/tag/v19.0.0
- 7: https://react.dev/blog/2024/04/25/react-19-upgrade-guide
Handle static empty children and signed numeric literals.
getExpressionAsLiteralText does not classify Boolean, null, or signed numeric expressions, so <Say>{false}{null}{-1}</Say> creates an auto argument for the text -1 instead of extracting {-1} as the rendered value. React ignores Boolean and null JSX expression values while rendering numbers.
Return '' from Boolean and null literals, and process unary +/- numeric literals before the dynamic expression branch. Add a regression case for <Say>{false}{null}{-1}</Say> whose extracted message contains only {0} with value -1.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/transform-jsx/src/parser.ts` around lines 40 - 51, Update
getExpressionAsLiteralText to return an empty string for Boolean and null
literals, and recognize unary plus/minus numeric literals before dynamic
expression handling. Ensure the children reduction in parseJSXElement extracts
<Say>{false}{null}{-1}</Say> as a single argument with identifier 0 and value
-1, and add the corresponding regression test.
There was a problem hiding this comment.
Skipping, deliberately.
{null} and {false} were considered and declined by the maintainer earlier in this change, alongside the {10} handling that did land — the two were weighed together and only the numeric case was wanted. They render as nothing and currently extract as placeholders, which is a real inconsistency, just not one being fixed here.
Unary -1 is the same call: {10} covers a literal number written into a sentence, which is the case that shows up in real messages. {-1} as a translatable JSX child is vanishingly rare, and handling it opens the neighbouring questions of {+1}, {1e3}, and constant-folded expressions like {1 + 1} — none of which have an obvious stopping point. Left as a placeholder rather than half-answered.
No regression test added, since the behaviour asserted would be the behaviour we chose not to have.
There was a problem hiding this comment.
@k0d13, understood. I will not treat Boolean, null, or signed numeric JSX expression children as required literal extraction in this PR.
The current placeholder behaviour is an intentional scope boundary. No regression test is needed for the declined behaviour.
(ᵕ—ᴗ—)
✏️ Learnings added
Learnt from: k0d13
Repo: k0d13/saykit PR: 84
File: packages/transform-jsx/src/parser.ts:40-51
Timestamp: 2026-08-04T09:09:04.726Z
Learning: In `packages/transform-jsx/src/parser.ts`, `getExpressionAsLiteralText` intentionally treats only unsigned `NumericLiteral` JSX children as static numeric text. Boolean and null literals, signed numeric expressions such as `{-1}`, and other expressions remain placeholders, even though some render as empty text or a number in React. This is an explicit scope boundary for JSX message extraction.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
…om the message format
github-actions
Bot
added
the
package: transform-js
1 participant
JSX whitespace was normalised by a heuristic that guessed what a line break meant. It guessed wrong often enough to be unusable — a break against an element became a space you could not remove, so an element and the text after it could never be rendered inline.
Extraction now mirrors what JSX renders, via
t.react.buildChildren— the same function the JSX transform itself compiles children with, which runs Babel'scleanJSXElementLiteralChildinternally. The rule is JSX's own: a line break and the indentation around it are layout and disappear, two lines that both hold text rejoin with the single space that separated the words, and whatever is written inside a line is content.A space that has to survive a break is written
{' '}, which Prettier already inserts when it wraps a line ending in one. It extracts as a plain space rather than as a placeholder, and the same holds for any literal child —{'\n'},{`\n`},{10}— since a literal is text that renders. The old punctuation special case is gone: a.stranded on its own line now attaches with no space because that is how it renders, so the rule it was hand-coding no longer has to exist.Braces in literal text
Found while testing the above, and pre-existing on the JS side. Literal text reached the catalogue unescaped, so a message that meant a brace became an argument nobody supplies:
convertMessageToIcunow quotes braces as ICU's own'{'. An apostrophe is the character doing that quoting, so it is doubled only where ICU would read it as opening a quote — in front of a brace or another apostrophe — which leavesIt'salone rather than rewriting the id of every message that contains one.#is untouched: inside a plural that is the number being formatted.The escaping is asserted from both ends.
convert.test.tspins the strings produced, andpackages/integrationfeeds those exact strings back through the runtime formatter and asserts each returns as the text it started as.Edge whitespace
convertMessageToIcuno longer trims. Collapsing a message's indentation is the parser's job and it does it the way JSX does; by the time text arrives at the converter it is the text the message means, edges included, so{' '}now works at the start and end of a message too.Blast radius
Three example apps relied on the old lenient spacing. They gained
{' '}at exactly the breaks that used to supply a space, so every catalogue across all nine examples re-extracts byte-identical — no msgid hash moved, no translation orphaned.Docs updated in both places that told you to avoid
{' '}, which was exactly backwards, plus a new whitespace section and a braces section.541 tests passing,
turbo run checkclean, 100% coverage on both changed files.Summary by CodeRabbit
Bug Fixes
#formatting.Documentation