fix(js_formatter): avoid introducing linebreaks for single line string interpolations #2500
Conversation
github-actions
bot
added
A-Formatter
CodSpeed Performance ReportMerging #2500 will not alter performanceComparing Summary
|
ah-yu
force-pushed
the
str_template_fmt
branch
2 times, most recently
from
fba1854 to
9a4f42f
Compare
| : ""} | ||
| - @media (max-width: ${(props) => | ||
| - props.noBreakPoint ? "0" : constants.layout.breakpoint.break1}px) { | ||
| + @media (max-width: ${(props) => props.noBreakPoint ? "0" : constants.layout.breakpoint.break1}px) { |
There was a problem hiding this comment.
This mismatch occurs because Prettier treats the template string as CSS since the template string is prefixed with styled.div but Biome treats it as a normal template string.
| +)} <script>const tpl = html\`<div>\${innerExpr( 1 )} ${outerExpr( | ||
| + 2, | ||
| +)}</div>\`</script>`; | ||
| +const nestedFun = /* HTML */ `${outerExpr(1)} <script>const tpl = html\`<div>\${innerExpr( 1 )} ${outerExpr(2)}</div>\`</script>`; |
There was a problem hiding this comment.
All mismatches occur in this file because Prettier treats these template strings as HTML since the template string has the leading comment /* HTML */ but Biome treats them as normal template strings.
crates/biome_formatter/src/buffer.rs
Outdated
| @@ -597,6 +608,41 @@ fn clean_interned( | |||
| } | |||
| } | |||
|
|
|||
| fn clean_best_fitting( | |||
There was a problem hiding this comment.
We should document a bit of the logic. For example, we want to document what defines "being inside an expanded conditional content". (e.g. is_in_expanded_conditional_content)
There was a problem hiding this comment.
This function is for quick verification purposes, forgot to remove it. I've adjusted the logic related to RemoveSoftLinesBuffer a bit in 99330db. Please take a look
| @@ -179,6 +216,10 @@ impl AnyTemplateElement { | |||
| AnyTemplateElement::TsTemplateElement(template) => template.r_curly_token(), | |||
| } | |||
| } | |||
|
|
|||
| fn has_new_line_in_range(&self) -> bool { | |||
| self.syntax().text().contains_char('\n') | |||
There was a problem hiding this comment.
This won't work on Windows, where newlines can be \r. Also, wouldn't you risk hitting false positives, where you might have a string literal that contains a literal new line? e.g:
const f = `some \n \n ${foo} \n bar`;Wouldn't it be safer to extract the trivia and check for TriviaKind::Whitespace that contain newlines?
There was a problem hiding this comment.
Good point. Fixed in 43493fa
Summary
Closes #2470
In the newest version of
Prettier, efforts have been made to optimize the formatting of string interpolations within template literals, aiming to keep them on a single line whenever possible. However, there are two exceptions to this rule:hard_line_breakTest Plan
[email protected], To align with the prettier snapshots, we need to upgrade prettier to the latest version. chore: upgrade prettier and update snapshots #2502Add new tests and update corresponding snapshots
Note: This PR changes the behavior of formatting string interpolations within template literals so some existing snapshots need to be updated.