fix(react): escape newlines in JSX quoted attribute values#11728
fix(react): escape newlines in JSX quoted attribute values#11728teee32 wants to merge 4 commits intoswc-project:mainfrom
Conversation
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ca1a5c811
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '\n' => buf.push_str("\\n"), | ||
| '\r' => buf.push_str("\\r"), |
There was a problem hiding this comment.
\\n text
Str.value holds the semantic string contents, not already-escaped source text. Replacing \n/\r with the two-character sequences "\\n" and "\\r" here means the emitter will escape those backslashes again in crates/swc_ecma_codegen/src/lit.rs:489-490, so <div data-anything="a b" /> now lowers to a JS literal like "a\\nb" whose runtime value is backslash-n, not an actual newline. This affects every quoted JSX attribute containing a line break, so the change still produces incorrect props for the regression it is trying to fix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Fixes newline handling for quoted JSX attribute string values in the React JSX transform to avoid collapsing line breaks into spaces (issue #11550), aiming to preserve correct runtime string semantics and prevent hydration mismatches.
Changes:
- Updates
transform_jsx_attr_strin the React JSX transform to handle\n/\rdifferently from\t. - Adds a new regression fixture for issue #11550.
- Updates multiple fixture expected outputs to reflect the new newline behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
crates/swc_ecma_transforms_react/src/jsx/mod.rs |
Changes JSX attribute string transformation for \n/\r/\t. |
crates/swc_ecma_transforms_react/tests/jsx/fixture/issue-11550/input.js |
Adds regression input covering newline in quoted JSX attr. |
crates/swc_ecma_transforms_react/tests/jsx/fixture/issue-11550/output.mjs |
Adds regression expected output (currently reflects double-escaped newline). |
crates/swc_ecma_transforms_react/tests/jsx/fixture/vercel/1/output.mjs |
Updates expected output for multiline quoted attr values. |
crates/swc/tests/fixture/codegen/jsx-1/output/index.js |
Updates expected output for multiline quoted attr values. |
crates/swc/tests/fixture/issues-1xxx/1233/case-1/output/index.js |
Updates expected output for newline in quoted attr value. |
crates/swc/tests/fixture/issues-2xxx/2162/case4/output/index.js |
Updates expected output for backslash + newline sequence in quoted attr value. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| '\n' => buf.push_str("\\n"), | ||
| '\r' => buf.push_str("\\r"), |
There was a problem hiding this comment.
transform_jsx_attr_str is inserting the two-character sequences \\n / \\r into Str.value. The code generator already escapes actual line terminators in string values (see get_quoted_utf16 handling of LINE_FEED/CARRIAGE_RETURN), so this change causes double-escaping and changes runtime semantics (output becomes "\\n" which is a backslash+n, not a newline). Instead, keep \n/\r as actual characters in the string value (e.g., push the newline/carriage-return codepoints) and let codegen emit the escape sequences.
| '\n' => buf.push_str("\\n"), | |
| '\r' => buf.push_str("\\r"), | |
| '\n' => buf.push_char('\n'), | |
| '\r' => buf.push_char('\r'), |
| // Newline in quoted JSX attribute value should be escaped, not collapsed to space | ||
| // https://github.com/swc-project/swc/issues/11550 | ||
| const hello = /*#__PURE__*/ React.createElement("div", { | ||
| "data-anything": "bruh\\nbruh" |
There was a problem hiding this comment.
This fixture output currently contains \\n inside a JS string literal (e.g., "bruh\\nbruh"), which represents a literal backslash+n at runtime. If the intent is to preserve an actual newline from the JSX attribute, the emitted JS source should contain \n (single backslash) escape, not \\n. Update expected output after fixing the transform to avoid double-escaping.
| "data-anything": "bruh\\nbruh" | |
| "data-anything": "bruh\nbruh" |
| className: b, | ||
| header: "C", | ||
| subheader: "D E" | ||
| subheader: "D\\n E" |
There was a problem hiding this comment.
The expected output string literal uses \\n (double backslash), which encodes a literal \n sequence at runtime. For preserving a newline from the JSX attribute, the JS output should contain \n (single backslash) so that runtime value contains an actual line feed. Update this fixture expectation after adjusting the transform to avoid double-escaping.
| subheader: "D\\n E" | |
| subheader: "D\n E" |
| function Component() { | ||
| return /*#__PURE__*/ React.createElement("div", { | ||
| name: "A B" | ||
| name: "A\\n B" |
There was a problem hiding this comment.
The expected output string literal uses \\n (double backslash), which yields a literal backslash+n in the runtime string. If the JSX attribute contains a real newline, the emitted JS should use the \n escape (single backslash) so the runtime value contains a line feed. Update this fixture after fixing the transform to prevent double-escaping.
| name: "A\\n B" | |
| name: "A\n B" |
| function test() { | ||
| return /*#__PURE__*/ React.createElement(React.Fragment, null, /*#__PURE__*/ React.createElement(A, { | ||
| b: "\\ " | ||
| b: "\\\\n " |
There was a problem hiding this comment.
This expectation currently emits b: "\\\\n " (four backslashes then n), which results in a runtime string starting with \\n... (literal backslashes + n), not a backslash followed by a newline. After fixing newline handling in the JSX attr transform, this fixture should expect an actual newline escape (\n) in the emitted JS rather than a doubly-escaped sequence.
| b: "\\\\n " | |
| b: "\n " |
| className: b, | ||
| header: "C", | ||
| subheader: "D E" | ||
| subheader: "D\\n E" |
There was a problem hiding this comment.
The expected output uses \\n (double backslash) in the string literal for subheader, which encodes a literal backslash+n at runtime. To preserve the newline from the JSX attribute value, the JS output should contain a single \n escape so runtime string contains an actual line feed. Update this fixture expectation after fixing the transform to avoid double-escaping.
| subheader: "D\\n E" | |
| subheader: "D\n E" |
|
GG ZIBLAKING seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Binary Sizes
Commit: a05cb73 |
Newlines in quoted JSX attribute string values were incorrectly collapsed to a single space in `transform_jsx_attr_str()`. Literal newlines in JS strings must be escaped as `\n`, not collapsed to whitespace. Fix: split the combined `'\n' | '\r' | '\t'` match arm into separate arms, escaping `\n` as `\\n` and `\r` as `\\r` while preserving the original `\t`-collapsing behaviour (tabs are horizontal whitespace and are intentionally collapsed to a space). Closes swc-project#11550. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
teee32
force-pushed
the
fix/jsx-attr-newline-escape
branch
from
1ca1a5c to
ff7676a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff7676ab91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '\t' => { | ||
| buf.push_char(' '); | ||
|
|
||
| while let Some(next) = iter.peek() { | ||
| if next.to_char() == Some(' ') { | ||
| iter.next(); |
There was a problem hiding this comment.
When a quoted JSX attribute spans lines in a tab-indented file, this branch still rewrites \t to a plain space and drops any following spaces. After the new \n/\r handling, an input like <div attr="a\n\tb" /> now lowers to a runtime value of "a\n b" instead of preserving the original tab, so multiline attribute strings are still corrupted for projects that indent JSX with tabs.
Useful? React with 👍 / 👎.
| '\t' => { | ||
| buf.push_char(' '); | ||
|
|
||
| while let Some(next) = iter.peek() { |
There was a problem hiding this comment.
Multiline JSX attribute values still normalize \t to a space in this branch.
Because this arm does buf.push_char(' ') (and then consumes following spaces), an input like <A x="a\n\tb" /> is lowered to a runtime value of "a\n b" instead of preserving the tab ("a\n\tb", matching TypeScript/Babel behavior).
So the newline fix is correct, but tab-indented multiline attrs are still semantically changed. Could we preserve \t here (for example buf.push_char('\t')) and add a fixture for that case?
| ' ' => buf.push_char(' '), | ||
|
|
||
| '\n' | '\r' | '\t' => { | ||
| '\n' => buf.push_char('\n'), |
There was a problem hiding this comment.
Nice fix for LF, but this exposes a CRLF regression. In read_jsx_str (crates/swc_ecma_parser/src/lexer/mod.rs), when read_jsx_new_line(false) consumes \r\n, chunk_start is still set via cur_pos + BytePos(ch.len_utf8() as _) (line 1505), so the \n gets re-sliced and appended again. I reproduced this by changing the fixture input newline to CRLF: the emitted value becomes "bruh\\r\\n\\nbruh" (extra LF). Could we advance chunk_start to the current lexer position after consuming the newline sequence and add a CRLF fixture?
| ' ' => buf.push_char(' '), | ||
|
|
||
| '\n' | '\r' | '\t' => { | ||
| '\n' => buf.push_char('\n'), |
There was a problem hiding this comment.
This change makes multiline JSX attribute strings diverge from Babel semantics. Babel applies value.value.replace(/\n\s+/g, " ") in @babel/plugin-transform-react-jsx, so inputs like <div a="x\n y" /> compile to "x y" (newline + indentation collapsed), while <div a="x\ny" /> keeps the newline. With unconditional '\n' => buf.push_char('\n') here, SWC now emits "x\n y", changing runtime values for existing indented multiline attributes and regressing fixtures such as codegen/jsx-1, issues-1233, and issues-2162. Could we preserve \n only when it is not followed by horizontal whitespace, and keep collapsing \n[ \t]+ to a single space?
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem
Literal newlines inside quoted JSX attribute string values are incorrectly
collapsed to a single space in the JSX transform. For example:
```jsx
// Input
// Current (wrong) output
React.createElement("div", { "data-anything": "line1 line2" }, "hello");
// Expected output
React.createElement("div", { "data-anything": "line1\nline2" }, "hello");
```
This causes real-world issues such as hydration mismatches in Next.js
applications (see #11550)
where data attributes containing newlines are corrupted.
Root Cause
In
transform_jsx_attr_str()atcrates/swc_ecma_transforms_react/src/jsx/mod.rs, the'\n' | '\r' | '\t'match arm collapses all three characters to a single space, consuming any
trailing spaces:
```rust
'\n' | '\r' | '\t' => {
buf.push_char(' ');
while let Some(next) = iter.peek() {
if next.to_char() == Some(' ') {
iter.next();
} else {
break;
}
}
}
```
Literal
\nand\rin JavaScript strings must be represented as escapesequences (
\n,\r), not collapsed to whitespace. Previous PR attempts(#11556,
#11569) tried pushing
literal characters but the outputs were interpreted as invalid JS (literal
newlines in string literals are a syntax error). The codegen at
crates/swc_ecma_codegen/src/lit.rsalready handles escaping LINE_FEEDand CARRIAGE_RETURN — the transform should push actual newline characters
and let codegen produce the correct escape sequences.
Change
Split the combined
'\n' | '\r' | '\t'match arm into three separate arms:```rust
'\n' => buf.push_char('\n'), // Push actual char; codegen escapes to \\n
'\r' => buf.push_char('\r'), // Push actual char; codegen escapes to \\r
'\t' => {
buf.push_char(' '); // kept as-is
while let Some(next) = iter.peek() {
if next.to_char() == Some(' ') {
iter.next();
} else {
break;
}
}
}
```
'\n': now pushes the actual newline codepoint; the existing codegen atlit.rs:489handles escaping it to\\n(single-backslash escape)'\r': same — codegen atlit.rs:490escapes to\\r'\t': behaviour unchanged — tabs are horizontal whitespace and areintentionally collapsed to a single space with trailing-space consumption
Why This Fix
existing escape logic (designed precisely for this purpose) handles the
output correctly
\nescape sequences, whichare syntactically valid in JS string literals
Risk
transform_jsx_attr_str()which is onlycalled during JSX attribute value transformation
updated and verified
Validation
cargo test -p swc_ecma_transforms_react jsx— 227 tests passcargo test -p swc --test projects— 869 tests passcargo clippy -p swc_ecma_transforms_react— no warningscargo fmt --all— formattedTest Coverage
tests/jsx/fixture/issue-11550/— regression test for thereported case
codegen/jsx-1,issues-1233/case-1,issues-2162/case4,vercel/1Closes #11550