fix(js_formatter): avoid introducing linebreaks for single line strin…

…g interpolations (#2500)

CHANGELOG.md

@@ -34,6 +34,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 ### Formatter
 
+#### Bug fixes
+
+- Fix [#2470](https://github.com/biomejs/biome/issues/2470) by avoid introducing linebreaks in single line string interpolations. Contributed by @ah-yu
+
 ### JavaScript APIs
 
 ### Linter

crates/biome_formatter/src/buffer.rs

@@ -1,5 +1,6 @@
 use super::{write, Arguments, FormatElement};
 use crate::format_element::Interned;
+use crate::prelude::tag::Condition;
 use crate::prelude::{LineMode, PrintMode, Tag};
 use crate::{Format, FormatResult, FormatState};
 use rustc_hash::FxHashMap;
@@ -488,10 +489,8 @@ pub struct RemoveSoftLinesBuffer<'a, Context> {
     /// that are now unused. But there's little harm in that and the cache is cleaned when dropping the buffer.
     interned_cache: FxHashMap<Interned, Interned>,
 
-    /// Marker for whether a `StartConditionalContent(mode: Expanded)` has been
-    /// written but not yet closed. Expanded content gets removed from this
-    /// buffer, so anything written while in this state simply gets dropped.
-    is_in_expanded_conditional_content: bool,
+    /// Store the conditional content stack to help determine if the current element is within expanded conditional content.
+    conditional_content_stack: Vec<Condition>,
 }
 
 impl<'a, Context> RemoveSoftLinesBuffer<'a, Context> {
@@ -500,20 +499,34 @@ impl<'a, Context> RemoveSoftLinesBuffer<'a, Context> {
         Self {
             inner,
             interned_cache: FxHashMap::default(),
-            is_in_expanded_conditional_content: false,
+            conditional_content_stack: Vec::new(),
         }
     }
 
     /// Removes the soft line breaks from an interned element.
     fn clean_interned(&mut self, interned: &Interned) -> Interned {
-        clean_interned(interned, &mut self.interned_cache)
+        clean_interned(
+            interned,
+            &mut self.interned_cache,
+            &mut self.conditional_content_stack,
+        )
+    }
+
+    /// Marker for whether a `StartConditionalContent(mode: Expanded)` has been
+    /// written but not yet closed.
+    fn is_in_expanded_conditional_content(&self) -> bool {
+        self.conditional_content_stack
+            .iter()
+            .last()
+            .is_some_and(|condition| condition.mode == PrintMode::Expanded)
     }
 }
 
 // Extracted to function to avoid monomorphization
 fn clean_interned(
     interned: &Interned,
     interned_cache: &mut FxHashMap<Interned, Interned>,
+    condition_content_stack: &mut Vec<Condition>,
 ) -> Interned {
     match interned_cache.get(interned) {
         Some(cleaned) => cleaned.clone(),
@@ -524,20 +537,18 @@ fn clean_interned(
                 .iter()
                 .enumerate()
                 .find_map(|(index, element)| match element {
-                    FormatElement::Line(LineMode::Soft | LineMode::SoftOrSpace) => {
-                        let mut cleaned = Vec::new();
-                        cleaned.extend_from_slice(&interned[..index]);
-                        Some((cleaned, &interned[index..]))
-                    }
-                    FormatElement::Tag(Tag::StartConditionalContent(condition))
-                        if condition.mode == PrintMode::Expanded =>
-                    {
+                    FormatElement::Line(LineMode::Soft | LineMode::SoftOrSpace)
+                    | FormatElement::Tag(
+                        Tag::StartConditionalContent(_) | Tag::EndConditionalContent,
+                    )
+                    | FormatElement::BestFitting(_) => {
                         let mut cleaned = Vec::new();
                         cleaned.extend_from_slice(&interned[..index]);
                         Some((cleaned, &interned[index..]))
                     }
                     FormatElement::Interned(inner) => {
-                        let cleaned_inner = clean_interned(inner, interned_cache);
+                        let cleaned_inner =
+                            clean_interned(inner, interned_cache, condition_content_stack);
 
                         if &cleaned_inner != inner {
                             let mut cleaned = Vec::with_capacity(interned.len());
@@ -548,41 +559,56 @@ fn clean_interned(
                             None
                         }
                     }
-
                     _ => None,
                 });
 
             let result = match result {
                 // Copy the whole interned buffer so that becomes possible to change the necessary elements.
                 Some((mut cleaned, rest)) => {
-                    let mut is_in_expanded_conditional_content = false;
-                    for element in rest {
-                        let element = match element {
-                            FormatElement::Tag(Tag::StartConditionalContent(condition))
-                                if condition.mode == PrintMode::Expanded =>
-                            {
-                                is_in_expanded_conditional_content = true;
+                    let mut element_stack = rest.iter().rev().collect::<Vec<_>>();
+                    while let Some(element) = element_stack.pop() {
+                        match element {
+                            FormatElement::Tag(Tag::StartConditionalContent(condition)) => {
+                                condition_content_stack.push(condition.clone());
                                 continue;
                             }
-                            FormatElement::Tag(Tag::EndConditionalContent)
-                                if is_in_expanded_conditional_content =>
-                            {
-                                is_in_expanded_conditional_content = false;
+                            FormatElement::Tag(Tag::EndConditionalContent) => {
+                                condition_content_stack.pop();
                                 continue;
                             }
                             // All content within an expanded conditional gets dropped. If there's a
                             // matching flat variant, that will still get kept.
-                            _ if is_in_expanded_conditional_content => continue,
+                            _ if condition_content_stack
+                                .iter()
+                                .last()
+                                .is_some_and(|condition| condition.mode == PrintMode::Expanded) =>
+                            {
+                                continue
+                            }
 
                             FormatElement::Line(LineMode::Soft) => continue,
-                            FormatElement::Line(LineMode::SoftOrSpace) => FormatElement::Space,
+                            FormatElement::Line(LineMode::SoftOrSpace) => {
+                                cleaned.push(FormatElement::Space)
+                            }
 
                             FormatElement::Interned(interned) => {
-                                FormatElement::Interned(clean_interned(interned, interned_cache))
+                                cleaned.push(FormatElement::Interned(clean_interned(
+                                    interned,
+                                    interned_cache,
+                                    condition_content_stack,
+                                )))
+                            }
+                            // Since this buffer aims to simulate infinite print width, we don't need to retain the best fitting.
+                            // Just extract the flattest variant and then handle elements within it.
+                            FormatElement::BestFitting(best_fitting) => {
+                                let most_flat = best_fitting.most_flat();
+                                most_flat
+                                    .iter()
+                                    .rev()
+                                    .for_each(|element| element_stack.push(element));
                             }
-                            element => element.clone(),
+                            element => cleaned.push(element.clone()),
                         };
-                        cleaned.push(element)
                     }
 
                     Interned::new(cleaned)
@@ -601,47 +627,42 @@ impl<Context> Buffer for RemoveSoftLinesBuffer<'_, Context> {
     type Context = Context;
 
     fn write_element(&mut self, element: FormatElement) -> FormatResult<()> {
-        let element = match element {
-            FormatElement::Tag(Tag::StartConditionalContent(condition)) => {
-                match condition.mode {
-                    PrintMode::Expanded => {
-                        // Mark that we're within expanded content so that it
-                        // can be dropped in all future writes until the ending
-                        // tag.
-                        self.is_in_expanded_conditional_content = true;
-                        return Ok(());
-                    }
-                    PrintMode::Flat => {
-                        // Flat groups have the conditional tag dropped as
-                        // well, since the content within it will _always_ be
-                        // printed by this buffer.
-                        return Ok(());
-                    }
-                }
-            }
-            FormatElement::Tag(Tag::EndConditionalContent) => {
-                // NOTE: This assumes that conditional content cannot be nested.
-                // This is true for all practical cases, but it's _possible_ to
-                // write IR that breaks this.
-                self.is_in_expanded_conditional_content = false;
-                // No matter if this was flat or expanded content, the ending
-                // tag gets dropped, since the starting tag was also dropped.
-                return Ok(());
-            }
-            // All content within an expanded conditional gets dropped. If there's a
-            // matching flat variant, that will still get kept.
-            _ if self.is_in_expanded_conditional_content => return Ok(()),
+        let mut element_statck = Vec::new();
+        element_statck.push(element);
 
-            FormatElement::Line(LineMode::Soft) => return Ok(()),
-            FormatElement::Line(LineMode::SoftOrSpace) => FormatElement::Space,
+        while let Some(element) = element_statck.pop() {
+            match element {
+                FormatElement::Tag(Tag::StartConditionalContent(condition)) => {
+                    self.conditional_content_stack.push(condition.clone());
+                }
+                FormatElement::Tag(Tag::EndConditionalContent) => {
+                    self.conditional_content_stack.pop();
+                }
+                // All content within an expanded conditional gets dropped. If there's a
+                // matching flat variant, that will still get kept.
+                _ if self.is_in_expanded_conditional_content() => continue,
 
-            FormatElement::Interned(interned) => {
-                FormatElement::Interned(self.clean_interned(&interned))
+                FormatElement::Line(LineMode::Soft) => continue,
+                FormatElement::Line(LineMode::SoftOrSpace) => {
+                    self.inner.write_element(FormatElement::Space)?
+                }
+                FormatElement::Interned(interned) => {
+                    let cleaned = self.clean_interned(&interned);
+                    self.inner.write_element(FormatElement::Interned(cleaned))?
+                }
+                // Since this buffer aims to simulate infinite print width, we don't need to retain the best fitting.
+                // Just extract the flattest variant and then handle elements within it.
+                FormatElement::BestFitting(best_fitting) => {
+                    let most_flat = best_fitting.most_flat();
+                    most_flat
+                        .iter()
+                        .rev()
+                        .for_each(|element| element_statck.push(element.clone()));
+                }
+                element => self.inner.write_element(element)?,
             }
-            element => element,
-        };
-
-        self.inner.write_element(element)
+        }
+        Ok(())
     }
 
     fn elements(&self) -> &[FormatElement] {

crates/biome_js_formatter/src/js/auxiliary/template_element.rs

@@ -6,11 +6,19 @@ use biome_formatter::{
 
 use crate::context::TabWidth;
 use crate::js::expressions::array_expression::FormatJsArrayExpressionOptions;
-use crate::js::lists::template_element_list::{TemplateElementIndention, TemplateElementLayout};
+use crate::js::lists::template_element_list::TemplateElementIndention;
 use biome_js_syntax::{
     AnyJsExpression, JsSyntaxNode, JsSyntaxToken, JsTemplateElement, TsTemplateElement,
 };
-use biome_rowan::{declare_node_union, AstNode, SyntaxResult};
+use biome_rowan::{declare_node_union, AstNode, NodeOrToken, SyntaxResult};
+
+enum TemplateElementLayout {
+    /// Tries to format the expression on a single line regardless of the print width.
+    SingleLine,
+
+    /// Tries to format the expression on a single line but may break the expression if the line otherwise exceeds the print width.
+    Fit,
+}
 
 #[derive(Debug, Clone, Default)]
 pub(crate) struct FormatJsTemplateElement {
@@ -44,8 +52,6 @@ declare_node_union! {
 
 #[derive(Debug, Copy, Clone, Default)]
 pub struct TemplateElementOptions {
-    pub(crate) layout: TemplateElementLayout,
-
     /// The indention to use for this element
     pub(crate) indention: TemplateElementIndention,
 
@@ -82,10 +88,33 @@ impl Format<JsFormatContext> for FormatTemplateElement {
             }
         });
 
-        let format_inner = format_with(|f: &mut JsFormatter| match self.options.layout {
+        let interned_expression = f.intern(&format_expression)?;
+
+        let layout = if !self.element.has_new_line_in_range() {
+            let will_break = if let Some(element) = &interned_expression {
+                element.will_break()
+            } else {
+                false
+            };
+
+            // make sure the expression won't break to prevent reformat issue
+            if will_break {
+                TemplateElementLayout::Fit
+            } else {
+                TemplateElementLayout::SingleLine
+            }
+        } else {
+            TemplateElementLayout::Fit
+        };
+
+        let format_inner = format_with(|f: &mut JsFormatter| match layout {
             TemplateElementLayout::SingleLine => {
                 let mut buffer = RemoveSoftLinesBuffer::new(f);
-                write!(buffer, [format_expression])
+
+                match &interned_expression {
+                    Some(element) => buffer.write_element(element.clone()),
+                    None => Ok(()),
+                }
             }
             TemplateElementLayout::Fit => {
                 use AnyJsExpression::*;
@@ -111,13 +140,21 @@ impl Format<JsFormatContext> for FormatTemplateElement {
                                 | JsLogicalExpression(_)
                                 | JsInstanceofExpression(_)
                                 | JsInExpression(_)
+                                | JsIdentifierExpression(_)
                         )
                     );
 
-                if indent {
-                    write!(f, [soft_block_indent(&format_expression)])
-                } else {
-                    write!(f, [format_expression])
+                match &interned_expression {
+                    Some(element) if indent => {
+                        write!(
+                            f,
+                            [soft_block_indent(&format_with(
+                                |f| f.write_element(element.clone())
+                            ))]
+                        )
+                    }
+                    Some(element) => f.write_element(element.clone()),
+                    None => Ok(()),
                 }
             }
         });
@@ -179,6 +216,20 @@ impl AnyTemplateElement {
             AnyTemplateElement::TsTemplateElement(template) => template.r_curly_token(),
         }
     }
+
+    fn has_new_line_in_range(&self) -> bool {
+        fn has_new_line_in_node(node: &JsSyntaxNode) -> bool {
+            node.children_with_tokens().any(|child| match child {
+                NodeOrToken::Token(token) => token
+                    // no need to check for trailing trivia as it's not possible to have a new line
+                    .leading_trivia()
+                    .pieces()
+                    .any(|trivia| trivia.is_newline()),
+                NodeOrToken::Node(node) => has_new_line_in_node(&node),
+            })
+        }
+        has_new_line_in_node(self.syntax())
+    }
 }
 
 /// Writes `content` with the specified `indention`.