fix(rome_js_formatter): in JSX, some spaces are removed rome#3211

Author: denbezrukov

crates/rome_js_formatter/src/jsx/lists/child_list.rs

@@ -6,6 +6,7 @@ use crate::utils::jsx::{
 use crate::JsFormatter;
 use rome_formatter::{format_args, write, FormatRuleWithOptions, VecBuffer};
 use rome_js_syntax::{JsxAnyChild, JsxChildList};
+use rome_rowan::TextSize;
 
 #[derive(Debug, Clone, Default)]
 pub struct FormatJsxChildList {
@@ -142,7 +143,7 @@ impl FormatRule<JsxChildList> for FormatJsxChildList {
                 // Any child that isn't text
                 JsxChild::NonText(non_text) => {
                     let line_mode = match children_iter.peek() {
-                        Some(JsxChild::Newline | JsxChild::Word(_) | JsxChild::Whitespace) => {
+                        Some(JsxChild::Word(word)) => {
                             // Break if the current or next element is a self closing element
                             // ```javascript
                             // <pre className="h-screen overflow-y-scroll" />adefg
@@ -152,16 +153,21 @@ impl FormatRule<JsxChildList> for FormatJsxChildList {
                             // <pre className="h-screen overflow-y-scroll" />
                             // adefg
                             // ```
-                            if matches!(non_text, JsxAnyChild::JsxSelfClosingElement(_)) {
+                            if matches!(non_text, JsxAnyChild::JsxSelfClosingElement(_))
+                                && word.len() != TextSize::from(1)
+                            {
                                 Some(LineMode::Hard)
                             } else {
                                 Some(LineMode::Soft)
                             }
                         }
 
                         // Add a hard line break if what comes after the element is not a text or is all whitespace
-                        Some(_) => Some(LineMode::Hard),
+                        Some(JsxChild::NonText(_)) => Some(LineMode::Hard),
 
+                        Some(JsxChild::Newline | JsxChild::Whitespace | JsxChild::EmptyLine) => {
+                            None
+                        }
                         // Don't insert trailing line breaks
                         None => None,
                     };

crates/rome_js_formatter/src/lib.rs

@@ -751,7 +751,8 @@ function() {
     // use this test check if your snippet prints as you wish, without using a snapshot
     fn quick_test() {
         let src = r#"
-        type C = B & (C | A) & B;
+const a = <><b/>a{' '}{" "}{" "}{" "}c{" "}{" "}{" "}{" "}{" "}{" "}</>;
+
 
 "#;
         let syntax = SourceType::tsx();

crates/rome_js_formatter/src/utils/jsx.rs

@@ -122,7 +122,7 @@ impl Format<JsFormatContext> for JsxSpace {
         write![
             formatter,
             [
-                if_group_breaks(&format_args![JsxRawSpace, hard_line_break()]),
+                if_group_breaks(&format_args![JsxRawSpace, soft_line_break()]),
                 if_group_fits_on_line(&space())
             ]
         ]
@@ -173,7 +173,7 @@ pub(crate) fn jsx_split_children<I>(children: I) -> SyntaxResult<Vec<JsxChild>>
 where
     I: IntoIterator<Item = JsxAnyChild>,
 {
-    let mut result = Vec::new();
+    let mut builder = JsxSplitChildrenBuilder::default();
 
     for child in children.into_iter() {
         match child {
@@ -203,15 +203,15 @@ where
                                     // </div>
                                     // ```
                                     if newlines > 1 {
-                                        result.push(JsxChild::EmptyLine);
+                                        builder.entry(JsxChild::EmptyLine);
                                     }
 
                                     continue;
                                 }
 
-                                result.push(JsxChild::Newline)
-                            } else if !matches!(result.last(), Some(JsxChild::Whitespace)) {
-                                result.push(JsxChild::Whitespace)
+                                builder.entry(JsxChild::Newline)
+                            } else {
+                                builder.entry(JsxChild::Whitespace)
                             }
                         }
                         _ => unreachable!(),
@@ -224,15 +224,15 @@ where
                             // Only handle trailing whitespace. Words must always be joined by new lines
                             if chunks.peek().is_none() {
                                 if whitespace.contains('\n') {
-                                    result.push(JsxChild::Newline);
+                                    builder.entry(JsxChild::Newline);
                                 } else {
-                                    result.push(JsxChild::Whitespace)
+                                    builder.entry(JsxChild::Whitespace)
                                 }
                             }
                         }
 
                         (relative_start, JsxTextChunk::Word(word)) => {
-                            result.push(JsxChild::Word(JsxWord {
+                            builder.entry(JsxChild::Word(JsxWord {
                                 text: value_token
                                     .token_text()
                                     .slice(TextRange::at(relative_start, word.text_len())),
@@ -245,23 +245,61 @@ where
 
             JsxAnyChild::JsxExpressionChild(child) => {
                 if is_whitespace_jsx_expression(&child) {
-                    match result.last() {
-                        Some(JsxChild::Whitespace) => {
-                            // Ignore
-                        }
-                        _ => result.push(JsxChild::Whitespace),
-                    }
+                    builder.entry(JsxChild::Whitespace)
                 } else {
-                    result.push(JsxChild::NonText(child.into()))
+                    builder.entry(JsxChild::NonText(child.into()))
                 }
             }
             child => {
-                result.push(JsxChild::NonText(child));
+                builder.entry(JsxChild::NonText(child));
+            }
+        }
+    }
+
+    Ok(builder.finish())
+}
+
+#[derive(Debug, Default)]
+struct JsxSplitChildrenBuilder {
+    pending: Option<JsxChild>,
+    buffer: Vec<JsxChild>,
+}
+
+impl JsxSplitChildrenBuilder {
+    fn is_last_whitespace(&self) -> bool {
+        matches!(self.buffer.last(), Some(JsxChild::Whitespace))
+    }
+
+    fn entry(&mut self, child: JsxChild) {
+        if let Some(pending) = self.pending.take() {
+            if matches!(
+                pending,
+                JsxChild::EmptyLine | JsxChild::Newline | JsxChild::Whitespace
+            ) {
+                if !matches!(child, JsxChild::Whitespace) && !self.is_last_whitespace() {
+                    self.buffer.push(pending)
+                }
+            } else {
+                self.buffer.push(pending);
             }
         }
+
+        self.pending = Some(child);
     }
 
-    Ok(result)
+    fn finish(mut self) -> Vec<JsxChild> {
+        if let Some(pending) = self.pending.take() {
+            if matches!(pending, JsxChild::Whitespace) {
+                if !self.is_last_whitespace() {
+                    self.buffer.push(pending)
+                }
+            } else {
+                self.buffer.push(pending);
+            }
+        }
+
+        self.buffer
+    }
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -328,6 +366,12 @@ pub(crate) struct JsxWord {
     source_position: TextSize,
 }
 
+impl JsxWord {
+    pub fn len(&self) -> TextSize {
+        self.text.len()
+    }
+}
+
 impl Format<JsFormatContext> for JsxWord {
     fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
         f.write_element(FormatElement::Text(Text::SyntaxTokenTextSlice {
@@ -594,6 +638,44 @@ mod tests {
         assert_eq!(children[3], JsxChild::Whitespace);
     }
 
+    #[test]
+    fn split_children_remove_in_row_jsx_whitespaces() {
+        let child_list = parse_jsx_children(r#"a{' '}{' '}{' '}c{" "}{' '}{" "}"#);
+
+        let children = jsx_split_children(&child_list).unwrap();
+
+        assert_eq!(
+            4,
+            children.len(),
+            "Expected to contain four elements. Actual:\n{children:#?} "
+        );
+        assert_word(&children[0], "a");
+        assert_eq!(children[1], JsxChild::Whitespace);
+        assert_word(&children[2], "c");
+        assert_eq!(children[3], JsxChild::Whitespace);
+    }
+
+    #[test]
+    fn split_children_remove_new_line_before_jsx_whitespaces() {
+        let child_list = parse_jsx_children(
+            r#"a
+            {' '}c{" "}
+            "#,
+        );
+
+        let children = jsx_split_children(&child_list).unwrap();
+
+        assert_eq!(
+            4,
+            children.len(),
+            "Expected to contain four elements. Actual:\n{children:#?} "
+        );
+        assert_word(&children[0], "a");
+        assert_eq!(children[1], JsxChild::Whitespace);
+        assert_word(&children[2], "c");
+        assert_eq!(children[3], JsxChild::Whitespace);
+    }
+
     fn assert_word(child: &JsxChild, text: &str) {
         match child {
             JsxChild::Word(word) => {

crates/rome_js_formatter/tests/specs/jsx/element.jsx

@@ -41,6 +41,75 @@ c = <div>a{' '}{' '}{' '}{' '}{' '}{' '}{' '}{' '}b{' '}{' '}{' '}{' '}{' '}{' '
 
 c2 = <div>a{' '}{' '}{' '}{' '}{' '}{' '}{' '}{' '}<div></div>content{' '}{' '}{' '}{' '}{' '}{' '}</div>;
 
+// this group should fit one line jsx whitespaces are hidden
+b =
+	<div>
+		<a></a>
+
+		{' '}
+
+		<a></a>
+
+		{' '}
+		1
+	</div>;
+
+// this group should break first tag and show only first jsx whitespace
+b1 =
+	<div>
+		<a>
+			{`
+12312
+12312
+			`}
+		</a>
+
+		{' '}
+
+		<a></a>
+
+		{' '}
+		1
+	</div>;
+
+// this group fit one line and hide jsx whitespace
+b2 =
+	<>
+		<a>123
+		</a>
+
+		{' '}
+		1
+	</>;
+
+// this group break group and show jsx whitespace
+b3 =
+	<>
+		<a>{`
+123`}
+		</a>
+
+		{' '}
+		1
+	</>;
+
+// one length text should insert soft line break
+b4 = <><pre />a</>;
+// longer than one length text should insert hard line break
+b5 = <><pre />ab</>;
+
+// one length text should insert soft line break show last jsx whitespace
+b6 = <><b/>a{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}</>;
+// longer than one length text should insert hard line break
+b7 = <><b/>longer{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}</>;
+
+const b8 = <div>
+	Text <a data-very-long-prop-breakline-rome-playground data-other>
+	some link
+</a>{' '}
+	| some other text,{' '}
+</div>;
+
 <div><div></div><a> jumps over the lazy dog </a></div>;
 
 const Essay = () => <div>The films of Wong Kar-Wai exemplify the synthesis of French New Wave cinema—specifically the unrelenting