Skip to content

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

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

5 participants