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

Normative: disallows UnicodeEscapeSequence #139

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Jack-Works
Copy link
Contributor

@Jack-Works Jack-Works commented Feb 25, 2022

For the following code:

import React from 'react'
const x = <video></\u0076ideo>

TypeScript: accept this syntax

TypeScript: reject this syntax after microsoft/TypeScript#48609

Babel: syntax error

swc: syntax error

esbuild: npx esbuild --loader=jsx <<< 'const x = <video></\u0076ideo>'

> <stdin>:1:19: error: Expected identifier but found "\\"
    1 │ const x = <video></\u0076ideo>
      ╵                    ^

Suggested change

Disallows UnicodeEscapeSequence in the JSXIdentifier

Another path

Allow UnicodeEscapeSequence but require them to match in the opening and the closing tag, which means <video></\u0076ideo> is a syntax error (tag mismatch) but <\u0076ideo /> is OK.

@wooorm
Copy link

wooorm commented Feb 25, 2022

npx esbuild --loader=jsx <<< 'const x = <video></\u0076ideo>'
 > <stdin>:1:19: error: Expected identifier but found "\\"
    1 │ const x = <video></\u0076ideo>
      ╵                    ^

1 error

spec.emu Outdated Show resolved Hide resolved
@wooorm wooorm mentioned this pull request Feb 25, 2022
@Huxpro Huxpro added Proposal Living Proposals considerable for the living JSX Normative Impl Reality Reality that the spec does not capture labels Feb 25, 2022
@Huxpro
Copy link
Contributor

Huxpro commented Feb 28, 2022

Thanks for catching this! Since implementation already diverged on this, I think we need some discussion and reach a consensus before making this change.

@Jack-Works
Copy link
Contributor Author

Actually only typescript accepts Unicode escape sequences. Maybe a LGTM from the Typescript team is enough.

@RyanCavanaugh
Copy link

Happy to disallow this from the TypeScript side

@Jack-Works
Copy link
Contributor Author

hi! Since TypeScript merges microsoft/TypeScript#48609, now this is consistent behavior across the most common transpilers in the ecosystem, that is disallows UnicodeEscapeSequence in JSXIdentifier. I think this PR can be merged now. I don't know who I can ping because Huxpro has left Meta. Maybe @sebmarkbage? Please take a look, thanks!

@Huxpro
Copy link
Contributor

Huxpro commented Oct 25, 2023

hi! Since TypeScript merges microsoft/TypeScript#48609, now this is consistent behavior across the most common transpilers in the ecosystem, that is disallows UnicodeEscapeSequence in JSXIdentifier. I think this PR can be merged now. I don't know who I can ping because Huxpro has left Meta. Maybe @sebmarkbage? Please take a look, thanks!

Thanks @Jack-Works! @sebmarkbage actually left before me 😂.

I'm supportive of this change. Maybe @poteto can take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Impl Reality Reality that the spec does not capture Normative Proposal Living Proposals considerable for the living JSX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants