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

[Fix] jsx-curly-brace-presence: make unwrapping single-expression template literals configurable #3608

Closed
wants to merge 1 commit into from

Conversation

akx
Copy link
Contributor

@akx akx commented Jul 21, 2023

Fixes #3607 (makes #3538 opt-in)

cc @taozhou-glean

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #3608 (f73bdab) into master (1a3a17a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3608   +/-   ##
=======================================
  Coverage   97.62%   97.62%           
=======================================
  Files         132      132           
  Lines        9295     9297    +2     
  Branches     3397     3399    +2     
=======================================
+ Hits         9074     9076    +2     
  Misses        221      221           
Impacted Files Coverage Δ
lib/rules/jsx-curly-brace-presence.js 98.13% <100.00%> (+0.01%) ⬆️

Copy link
Contributor

@taozhou-glean taozhou-glean left a comment

Choose a reason for hiding this comment

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

lgtm, defer to owner to approve

@@ -20,7 +20,7 @@ You can pass in options to enforce the presence of curly braces on JSX props, ch

```js
...
"react/jsx-curly-brace-presence": [<enabled>, { "props": <string>, "children": <string>, "propElementValues": <string> }]
"react/jsx-curly-brace-presence": [<enabled>, { "props": <string>, "children": <string>, "propElementValues": <string>, "unwrapTemplateLiterals": <boolean> }]
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe unwrapSingleExpressionOnlyTemplateLiterals just to be more explicit ?

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I think the proper fix here is just to revert #3538, but keep the test cases (without the autofixes).

### `unwrapTemplateLiterals`

- `true`: unwrap template literals that only have a single expression inside of them.
Since template literals return strings, this may cause changes in semantics, or type errors.
Copy link
Member

Choose a reason for hiding this comment

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

this makes it an unsafe autofix, which means it can't ever exist.

@akx
Copy link
Contributor Author

akx commented Jul 24, 2023

Closing in favor of the revert #3611.

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

Successfully merging this pull request may close these issues.

[Bug]: jsx-curly-brace-presence template literal fix can break things
3 participants