-
-
Notifications
You must be signed in to change notification settings - Fork 511
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(useSortedClasses): should suggest code fixes that match the JSX quote style of the formatter #4857
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR. I suggested few changes
crates/biome_analyze/src/context.rs
Outdated
@@ -17,6 +17,7 @@ pub struct RuleContext<'a, R: Rule> { | |||
file_path: &'a Path, | |||
options: &'a R::Options, | |||
preferred_quote: &'a PreferredQuote, | |||
preferred_jsx_quote: Option<&'a PreferredQuote>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like preferred_quote
, field (line 19) it shouldn't be a Option
crates/biome_analyze/src/context.rs
Outdated
self.preferred_jsx_quote | ||
.expect("preferred_jsx_quote should be provided") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you remove the Option
, you can remove expect
CHANGELOG.md
Outdated
@@ -15,6 +15,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b | |||
|
|||
### Analyzer | |||
|
|||
#### New features | |||
|
|||
- The analyzer now infers the correct JSX quote style from `javascript.formatter.jsxQuoteStyle`, so code fixes will use the same quote style as the formatter. Contributed by @lucasweng |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That isn't true, though. In this PR you just fixed useSortedClasses
, but there are many JSX-related rules that don't use this new feature yet. You're free to implement it in another PR if you want. As for now, we can remove this line
CodSpeed Performance ReportMerging #4857 will not alter performanceComparing Summary
|
Thank you @ematipico! I’ve made some updates based on your comments:2673601 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Summary
preferred_jsx_quote
to the analyzer. It is used only inside the JavaScript analyzer to ensure that code fixes match the JSX quote style specified in the formatter’s configuration.useSortedClasses
rule to use this new logic.Test Plan