facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.2k stars 46.32k forks source link

[Compiler Bug]: Stripping curly braces breaks character escapes (breaks JSX spec 1.5.1) #29648

Open coravacav opened 3 months ago

coravacav commented 3 months ago

What kind of issue is this?

Link to repro

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAgHgBwjALgAgBMEAzAQygBsCSoA7OXASwjvwAkIBbBACgEp8wADpt8MBLlhteo-PPwAeAHxyF8xQGVuCAMLdsdBHQK4MuALzAA5AEFhogKLWAvvgD0qseq079XQ2NTcwthEHsnMI8vH081fgBuURdREBcgA

Repro steps

  1. Use a string with a newline character (\n) passed as a prop to a component with curly braces prop={"text\ntext"}
  2. Observe that react compiler outputs prop="text\ntext" instead of prop={"text\ntext"}

This is incorrect behavior. The newline character is no longer properly treated as a newline but rather as the raw text \n.

You can see this behavior be correctly also incorrectly handled in the second component, where prop="text\ntext" becomes prop="text\\ntext", resulting in the wrong text being displayed on the screen.

JSX rules specifically don't allow this kind of conversion, noted in 1.5.1 of the spec

Historically, string characters within JSXAttributeValue and JSXText are extended to allow the presence of HTML character references to make copy-pasting between HTML and JSX easier, at the cost of not supporting \ EscapeSequence of ECMAScript's StringLiteral.

How often does this bug happen?

Every time

What version of React are you using?

19.0.0-rc-6f23540c7d-20240528

josephsavona commented 3 months ago

Thanks for reporting, this looks like a bug. We'll prioritize a fix.

SeekingLight233 commented 3 months ago
image

It seems like this issue existed even before BuildHIR🤔

poteto commented 2 months ago

Thanks for reporting! I have a PR #29997 for preserving the JsxExpressionContainer if it contains non-ascii characters, which fixes the first example in your repro. However the 2nd case is an issue with @babel/parser as @SeekingLight233 pointed out. We could perhaps also check if jsx attribute string literals contain those special characters and convert it into a JsxExpressionContainer