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: revert single template literal expression which might be intended usage #3611

Merged
merged 1 commit into from Jul 24, 2023

Conversation

taozhou-glean
Copy link
Contributor

@taozhou-glean taozhou-glean commented Jul 23, 2023

This reverts #3538, while keeping the test case there for explanation and future breakage prevention.

The previous commit broke for intended type casting usage: #3607

Sorry for adding the change, did not think enough before making it, apparently this is definitely legit use case for casting other type to string, we might be able to keep this rule and limit it to string single template literal with typechecker, but just a bit overkill at this moment.

@codecov
Copy link

codecov bot commented Jul 23, 2023

Codecov Report

Merging #3611 (9ea0010) into master (1a3a17a) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 9ea0010 differs from pull request most recent head f818096. Consider uploading reports for the commit f818096 to get more accurate results

@@            Coverage Diff             @@
##           master    #3611      +/-   ##
==========================================
- Coverage   97.62%   97.62%   -0.01%     
==========================================
  Files         132      132              
  Lines        9295     9288       -7     
  Branches     3397     3394       -3     
==========================================
- Hits         9074     9067       -7     
  Misses        221      221              
Impacted Files Coverage Δ
lib/rules/jsx-curly-brace-presence.js 98.04% <ø> (-0.07%) ⬇️

@ljharb
Copy link
Member

ljharb commented Jul 23, 2023

This has nothing to do with TS; JS also can use propTypes or runtime type checks - it's precisely as critical a use case in JS as it is in TS.

Honest mistake, though - I missed this too when reviewing your initial PR.

@taozhou-glean
Copy link
Contributor Author

This has nothing to do with TS; JS also can use propTypes or runtime type checks - it's precisely as critical a use case in JS as it is in TS.

Honest mistake, though - I missed this too when reviewing your initial PR.

right, updated the description.

@ljharb ljharb force-pushed the tao-revert-3538 branch 2 times, most recently from 698d6c7 to f818096 Compare July 24, 2023 05:17
@ljharb ljharb merged commit f818096 into jsx-eslint:master Jul 24, 2023
286 checks passed
@akx
Copy link
Contributor

akx commented Jul 24, 2023

Thanks, I didn't have the bandwidth over the weekend to do this in #3608 👍

@lucapollani
Copy link

Will this fix be released soon?

@ljharb
Copy link
Member

ljharb commented Jul 28, 2023

I'm sure it will.

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.

None yet

4 participants