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

correctly print exotic JSX names #6451

Merged
merged 2 commits into from
Oct 26, 2023
Merged

correctly print exotic JSX names #6451

merged 2 commits into from
Oct 26, 2023

Conversation

tsnobip
Copy link
Contributor

@tsnobip tsnobip commented Oct 25, 2023

before this fix,

<\"my-custom-tag" className="hello" />

would get reformatted to:

<my-custom-tag className="hello" />

which doesn't compile.

Copy link
Collaborator

@zth zth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! But I'm not very familiar with the parser/printer so someone else should take a look at it too. I'll request an additional review.

@@ -6,13 +6,17 @@ let x = <Foo.bar className="container" />
let x = <Foo.baz
className="multiline"
/>
let x = <\"custom-tag" className="container" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note, but given that this looks like a close tag (but is not), are there other issues this could cause? How can we test those?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes readability suffers a bit with this, maybe we could change the parser to make it accept exotic names inside double quotes without escaping them with a \, like polymorphic variants, but this would be more work for something that is definitely a corner case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably will be easier to make <my-custom-tag className="hello" /> compile in the case

@zth zth requested a review from cristianoc October 25, 2023 19:25
@cristianoc
Copy link
Collaborator

Looks good to me.

@zth
Copy link
Collaborator

zth commented Oct 26, 2023

@tsnobip can you add an entry to the changelog? Then we're good to go.

@zth zth merged commit cff0a81 into master Oct 26, 2023
14 checks passed
@zth zth deleted the exotic_jsx_names branch October 26, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants