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 template literal fix can break things #3607

Closed
2 tasks done
akx opened this issue Jul 21, 2023 · 4 comments
Closed
2 tasks done

[Bug]: jsx-curly-brace-presence template literal fix can break things #3607

akx opened this issue Jul 21, 2023 · 4 comments
Labels

Comments

@akx
Copy link
Contributor

akx commented Jul 21, 2023

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

#3538 (see #3537) fixes a template literal with a single placeholder to just be that placeholder.

This can change the semantics of the prop:

const foo = 123;
<Component foo={`${foo}`} />

casts foo to a string (and thus passes "123"), while the "fixed"

const foo = 123;
<Component foo={foo} />

passes foo as a number 123; when using TypeScript and Component expects foo: string, that's a type error. When not using TypeScript, if Component behaves differently with a string prop cf. a number prop, this changes that behavior.

This also stands for children:

<span>An error occurred: {`${err}`}</span>

will be changed to

<span>An error occurred: {err}</span>

which can be a type error if err is not a valid React child (and when not using TypeScript, it can cause e.g. false not to get rendered as text.

Expected Behavior

Whether template literals with a single placeholder are unwrapped should be configurable.

eslint-plugin-react version

v7.33.0

eslint version

v8.45.0

node version

v20.3.0

@akx akx added the bug label Jul 21, 2023
@akx
Copy link
Contributor Author

akx commented Jul 21, 2023

cc @taozhou-glean

@akx

This comment was marked as off-topic.

@taozhou-glean
Copy link
Contributor

did not realize use case like this until now, thanks for raising and fixing!

@ljharb
Copy link
Member

ljharb commented Jul 21, 2023

Indeed, I think #3538 needs to be reverted.

ljharb pushed a commit to taozhou-glean/eslint-plugin-react that referenced this issue Jul 23, 2023
ljharb pushed a commit to taozhou-glean/eslint-plugin-react that referenced this issue Jul 24, 2023
@ljharb ljharb closed this as completed in f818096 Jul 24, 2023
caesar1030 pushed a commit to caesar1030/eslint-plugin-react that referenced this issue Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants