Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
SukkaW opened this issue May 17, 2024 · 4 comments
Closed

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

SukkaW opened this issue May 17, 2024 · 4 comments
Labels
Component: Optimizing Compiler Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@SukkaW
Copy link

SukkaW commented May 17, 2024

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.

@SukkaW SukkaW added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label May 17, 2024
@josephsavona
Copy link
Contributor

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
Copy link
Author

SukkaW commented May 17, 2024

@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
Copy link
Contributor

himself65 commented May 17, 2024

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
Copy link
Contributor

@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.

josephsavona added a commit that referenced this issue May 17, 2024
Workaround for a bug in older versions of Babel, where strings with unicode are incorrectly escaped when emitted as JSX attributes, causing double-escaping by later processing.

Closes #29120
Closes #29124

[ghstack-poisoned]
josephsavona added a commit that referenced this issue May 17, 2024
Workaround for a bug in older versions of Babel, where strings with unicode are incorrectly escaped when emitted as JSX attributes, causing double-escaping by later processing.

Closes #29120
Closes #29124

ghstack-source-id: 065440d4fb97e164beb8a8f15f252f372a59c5a0
Pull Request resolved: #29141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Optimizing Compiler Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants