jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
9.01k stars 2.77k forks source link

[Bug]: `jsx-curly-brace-presence` still warns on strings containing unescaped entities #3801

Open musjj opened 3 months ago

musjj commented 3 months ago

Is there an existing issue for this?

Description Overview

This commit: https://github.com/jsx-eslint/eslint-plugin-react/commit/fe708e1936770bb66b74a473bf396e6714ccae36 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: https://github.com/jsx-eslint/eslint-plugin-react/issues/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 commented 2 months ago

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

musjj commented 2 months ago

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 commented 2 months ago

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

musjj commented 2 months ago

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 https://github.com/jsx-eslint/eslint-plugin-react/commit/fe708e1936770bb66b74a473bf396e6714ccae36).

ljharb commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 :thinking: ?

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 commented 2 months ago

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.