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

fix(compiler): avoid escape non-English character #29124

Closed

Conversation

himself65
Copy link
Contributor

Summary

Fixes: #29120

Related: babel/babel#9804

How did you test this change?

@himself65 himself65 force-pushed the himself65/fix-chinese-charater branch from ad29355 to b871f19 Compare May 17, 2024 07:14
@react-sizebot
Copy link

react-sizebot commented May 17, 2024

Comparing: 3f1436c...6464375

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB +0.11% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 495.01 kB 495.01 kB = 88.68 kB 88.68 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 499.81 kB 499.81 kB = 89.36 kB 89.36 kB
facebook-www/ReactDOM-prod.classic.js = 592.16 kB 592.16 kB = 104.15 kB 104.15 kB
facebook-www/ReactDOM-prod.modern.js = 568.39 kB 568.39 kB = 100.55 kB 100.55 kB
test_utils/ReactAllWarnings.js Deleted 64.26 kB 0.00 kB Deleted 16.02 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 64.26 kB 0.00 kB Deleted 16.02 kB 0.00 kB

Generated by 🚫 dangerJS against 6464375

Copy link

vercel bot commented May 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 17, 2024 7:40am

@himself65
Copy link
Contributor Author

I changed the behavior from modifying the babel config to changing StringLiteral to Template.

Some feedback from @SukkaW that we cannot modify babel config in many cases

@himself65 himself65 changed the title fix(compiler-playground): avoid escape non-English character fix(compiler): avoid escape non-English character May 17, 2024
@himself65
Copy link
Contributor Author

So many breaking changes to tests. I don't think this is a good change

@josephsavona
Copy link
Contributor

Thanks for submitting this! The fix is a bit more involved, we should only apply the workaround if necessary (if babel would escape incorrectly). I'm also going to investigate if there is some way to just get the right escaping in older versions of babel.

josephsavona added a commit that referenced this pull request 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 pull request 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: React Compiler should not encode in JSX prop value
5 participants