facebook / react

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

Bug: React Compiler should not encode in JSX prop value #29120

Closed SukkaW closed 3 weeks ago

SukkaW commented 4 weeks ago

React version: 19.0.0-beta-26f2496093-20240514

Steps To Reproduce

Link to code example:

Input Code

'use client';

function Comp() {
  return (
    <div aria-label="我能吞下玻璃而不伤身体">
      我能吞下玻璃而不伤身体
    </div>
  )
}

React Compiler Output

function Comp() {
  const $ = _c(1);

  let t0;

  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
    t0 = (
      <div aria-label="\u6211\u80FD\u541E\u4E0B\u73BB\u7483\u800C\u4E0D\u4F24\u8EAB\u4F53">
        我能吞下玻璃而不伤身体
      </div>
    );
    $[0] = t0;
  } else {
    t0 = $[0];
  }

  return t0;
}

React Compiler Playground

Babel / SWC / ESBuild JSX transform

SWC:

"use client";
import { jsx as _jsx } from "react/jsx-runtime";
function Comp() {
    var $ = _c(1);
    var t0;
    if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
        t0 = _jsx("div", {
            "aria-label": "\\u6211\\u80FD\\u541E\\u4E0B\\u73BB\\u7483\\u800C\\u4E0D\\u4F24\\u8EAB\\u4F53",
            children: "我能吞下玻璃而不伤身体"
        });
        $[0] = t0;
    } else {
        t0 = $[0];
    }
    return t0;
}

SWC Playground

Babel:

"use strict";
'use client';

var _jsxRuntime = require("react/jsx-runtime");
function Comp() {
  const $ = _c(1);
  let t0;
  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
    t0 = /*#__PURE__*/(0, _jsxRuntime.jsx)("div", {
      "aria-label": "\\u6211\\u80FD\\u541E\\u4E0B\\u73BB\\u7483\\u800C\\u4E0D\\u4F24\\u8EAB\\u4F53",
      children: "\u6211\u80FD\u541E\u4E0B\u73BB\u7483\u800C\u4E0D\u4F24\u8EAB\u4F53"
    });
    $[0] = t0;
  } else {
    t0 = $[0];
  }
  return t0;
}

Babel REPL

ESBuild:

'use client'

import { jsx } from "react/jsx-runtime";
function Comp() {
  const $ = _c(1);
  let t0;
  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
    t0 = /* @__PURE__ */ jsx("div", { "aria-label": "\\u6211\\u80FD\\u541E\\u4E0B\\u73BB\\u7483\\u800C\\u4E0D\\u4F24\\u8EAB\\u4F53", children: "\u6211\u80FD\u541E\u4E0B\u73BB\u7483\u800C\u4E0D\u4F24\u8EAB\u4F53" });
    $[0] = t0;
  } else {
    t0 = $[0];
  }
  return t0;
}

ESBuild REPL

The current behavior

The prop value becomes double escaped.

The expected behavior

The prop value should only be escaped once.

josephsavona commented 4 weeks ago

Thanks for filing! Per @himself65’s comment this does appear to stem from Babel. We’ve noticed other issues with Babel incorrectly escaping JSX string attributes. However we can workaround this in the compiler by trying to detect when Babel would escape incorrectly and emit code differently. We’ll take a look!

SukkaW commented 4 weeks ago

@josephsavona IMHO the most efficient and straight forward workaround would be like this:

// React Compiler emitting
<div aria-label={'\u6211\u80FD\u541E\u4E0B\u73BB\u7483\u800C\u4E0D\u4F24\u8EAB\u4F53'} />

This works with all existing JSX transformers (Babel, SWC, ESBuild).

himself65 commented 4 weeks ago

Thanks for filing! Per @himself65’s comment this does appear to stem from Babel. We’ve noticed other issues with Babel incorrectly escaping JSX string attributes. However we can workaround this in the compiler by trying to detect when Babel would escape incorrectly and emit code differently. We’ll take a look!

Yeah I think this from babel issue and here is a workaround for playground code

Subject: [PATCH] fix: files
---
Index: compiler/apps/playground/package.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/compiler/apps/playground/package.json b/compiler/apps/playground/package.json
--- a/compiler/apps/playground/package.json (revision 5ab54718a52d738dcbd03fcb43a556993b445ed4)
+++ b/compiler/apps/playground/package.json (date 1715928974123)
@@ -12,15 +12,15 @@
     "test": "playwright test"
   },
   "dependencies": {
-    "@babel/core": "^7.19.1",
-    "@babel/parser": "^7.19.1",
-    "@babel/plugin-syntax-typescript": "^7.18.6",
-    "@babel/plugin-transform-block-scoping": "^7.18.9",
-    "@babel/plugin-transform-modules-commonjs": "^7.18.6",
-    "@babel/preset-react": "^7.18.6",
-    "@babel/preset-typescript": "^7.18.6",
-    "@babel/traverse": "^7.19.1",
-    "@babel/types": "^7.19.0",
+    "@babel/core": "8.0.0-alpha.8",
+    "@babel/parser": "8.0.0-alpha.8",
+    "@babel/plugin-syntax-typescript": "8.0.0-alpha.8",
+    "@babel/plugin-transform-block-scoping": "8.0.0-alpha.8",
+    "@babel/plugin-transform-modules-commonjs": "8.0.0-alpha.8",
+    "@babel/preset-react": "8.0.0-alpha.8",
+    "@babel/preset-typescript": "8.0.0-alpha.8",
+    "@babel/traverse": "8.0.0-alpha.8",
+    "@babel/types": "8.0.0-alpha.8",
     "@heroicons/react": "^1.0.6",
     "@monaco-editor/react": "^4.4.6",
     "@playwright/test": "^1.42.1",
@@ -52,10 +52,10 @@
     "tailwindcss": "^3.2.4"
   },
   "resolutions": {
-    "./**/@babel/parser": "7.7.4",
-    "./**/@babel/types": "7.7.4",
-    "@babel/core": "7.2.0",
-    "@babel/traverse": "7.1.6",
-    "@babel/generator": "7.2.0"
+    "./**/@babel/parser": "8.0.0-alpha.8",
+    "./**/@babel/types": "8.0.0-alpha.8",
+    "@babel/core": "8.0.0-alpha.8",
+    "@babel/traverse": "8.0.0-alpha.8",
+    "@babel/generator": "8.0.0-alpha.8"
   }
 }
image
josephsavona commented 4 weeks ago

@SukkaW yup JsxExpressionContainer is the workaround we’ve been using for these types of Babel escaping issues. We can’t depend on 8 yet just because of the need for backward compatibility.