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]: jsx-curly-brace-presence still warns on strings containing unescaped entities #3801

Open
2 tasks done
musjj opened this issue Aug 14, 2024 · 9 comments
Open
2 tasks done
Labels

Comments

@musjj
Copy link

musjj commented Aug 14, 2024

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

This commit: fe708e1 adds a special case that is supposed to allow plain string literals inside JSX expressions if it contains unescaped HTML entities:

<p>{`'`}</p>

Previously, the warning will yield the following invalid JSX when "fixed":

<p>'</p>

The issue still remains because the special case only considers strings quoted with backticks but not normal quotes

Related issue: #3214

Expected Behavior

This should not trigger a warning:

<p>{"'"}</p>

eslint-plugin-react version

v7.34.0

eslint version

v8.57.0

node version

v20.15.1

@ljharb
Copy link
Member

ljharb commented Sep 11, 2024

<p>'</p> isn't invalid JSX tho?

@musjj
Copy link
Author

musjj commented Sep 11, 2024

Wow yeah, you're right. I had react/no-unescaped-entities on which made me thought that it was a hard syntax rule. But I think my issue still applies (the exception should apply to double quoted strings too).

@ljharb
Copy link
Member

ljharb commented Sep 11, 2024

<p>"</p> is also valid JSX, though - it's only in attributes that the curlies become necessary.

@musjj
Copy link
Author

musjj commented Sep 11, 2024

No I meant that <p>{"'"}</p> shouldn't trigger jsx-curly-brace-presence, just like how <p>{`'`}</p> triggers no lint warnings (thanks to the special exception in fe708e1).

@ljharb
Copy link
Member

ljharb commented Sep 11, 2024

hmm - why? because of no-unescaped entities?

straight quotes shouldn't ever appear in prose anyways, they're typographically incorrect - what's the actual context here for a lone single quote in a p tag?

@musjj
Copy link
Author

musjj commented Sep 11, 2024

straight quotes shouldn't ever appear in prose anyways, they're typographically incorrect

What do you mean? The error is still triggered in things like <p>{"What's up"}</p> (but not in <p>{`What's up`}</p>). Is there a reason why the exception should only apply to backquoted strings but not double-quoted ones?

@ljharb
Copy link
Member

ljharb commented Sep 11, 2024

Because template literals shouldn't be (and basically aren't) used for one line strings with no substitutions.

What's up is typographically incorrect - it should be What‘s up.

@musjj
Copy link
Author

musjj commented Sep 11, 2024

Because template literals shouldn't be (and basically aren't) used for one line strings with no substitutions.

Then why does <p>{`What's up`}</p> not throw any lint errors 🤔 ?

What's up is typographically incorrect - it should be What‘s up.

I don't think this nit is relevant to the issue tbh.

My main concern here is inconsistency. <p>{"What's up"}</p> throws an error, but <p>{`What's up`}</p> doesn't, which is really confusing. Either both of them should error out or both of them should pass.

@ljharb
Copy link
Member

ljharb commented Sep 11, 2024

I suppose we could adjust it so that one-line template literals with no substitutions still warn - but I'm skeptical anyone would benefit from that, because mostly nobody uses backticks for that case in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants