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

Author: denbezrukov

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

@@ -7,7 +7,6 @@ use crate::JsFormatter;
 use rome_formatter::format_element::tag::Tag;
 use rome_formatter::{format_args, write, CstFormatContext, FormatRuleWithOptions, VecBuffer};
 use rome_js_syntax::{JsxAnyChild, JsxChildList};
-use rome_rowan::TextSize;
 use std::cell::RefCell;
 
 #[derive(Debug, Clone, Default)]
@@ -186,7 +185,7 @@ impl FormatJsxChildList {
                             // adefg
                             // ```
                             if matches!(non_text, JsxAnyChild::JsxSelfClosingElement(_))
-                                && word.len() != TextSize::from(1)
+                                && !word.is_ascii_punctuation()
                             {
                                 Some(LineMode::Hard)
                             } else {

crates/rome_js_formatter/src/utils/jsx.rs

@@ -168,7 +168,7 @@ impl Format<JsFormatContext> for JsxSpace {
         write![
             formatter,
             [
-                if_group_breaks(&format_args![JsxRawSpace, soft_line_break()]),
+                if_group_breaks(&format_args![JsxRawSpace, hard_line_break()]),
                 if_group_fits_on_line(&space())
             ]
         ]
@@ -223,7 +223,7 @@ pub(crate) fn jsx_split_children<I>(
 where
     I: IntoIterator<Item = JsxAnyChild>,
 {
-    let mut builder = JsxSplitChildrenBuilder::default();
+    let mut builder = JsxSplitChildrenBuilder::new();
 
     for child in children.into_iter() {
         match child {
@@ -309,45 +309,31 @@ where
     Ok(builder.finish())
 }
 
-#[derive(Debug, Default)]
+/// The builder removes [JsxChild::EmptyLine], [JsxChild::Newline], [JsxChild::Whitespace]
+/// if a next element is [JsxChild::Whitespace]
+/// [Prettier applies]: https://github.com/prettier/prettier/blob/b0d9387b95cdd4e9d50f5999d3be53b0b5d03a97/src/language-js/print/jsx.js#L144-L180
+#[derive(Debug)]
 struct JsxSplitChildrenBuilder {
-    pending: Option<JsxChild>,
     buffer: Vec<JsxChild>,
 }
 
 impl JsxSplitChildrenBuilder {
-    fn is_last_whitespace(&self) -> bool {
-        matches!(self.buffer.last(), Some(JsxChild::Whitespace))
+    fn new() -> Self {
+        JsxSplitChildrenBuilder { buffer: vec![] }
     }
 
     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);
+        match self.buffer.last_mut() {
+            Some(last @ (JsxChild::EmptyLine | JsxChild::Newline | JsxChild::Whitespace))
+                if matches!(child, JsxChild::Whitespace) =>
+            {
+                *last = child
             }
+            _ => self.buffer.push(child),
         }
-
-        self.pending = Some(child);
     }
 
-    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);
-            }
-        }
-
+    fn finish(self) -> Vec<JsxChild> {
         self.buffer
     }
 }
@@ -417,8 +403,13 @@ pub(crate) struct JsxWord {
 }
 
 impl JsxWord {
-    pub fn len(&self) -> TextSize {
-        self.text.len()
+    pub fn is_ascii_punctuation(&self) -> bool {
+        self.text.chars().count() == 1
+            && self
+                .text
+                .chars()
+                .next()
+                .map_or(false, |char| char.is_ascii_punctuation())
     }
 }
 
@@ -693,7 +684,7 @@ mod tests {
     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();
+        let children = jsx_split_children(&child_list, &Comments::default()).unwrap();
 
         assert_eq!(
             4,
@@ -714,7 +705,7 @@ mod tests {
             "#,
         );
 
-        let children = jsx_split_children(&child_list).unwrap();
+        let children = jsx_split_children(&child_list, &Comments::default()).unwrap();
 
         assert_eq!(
             4,

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

@@ -93,17 +93,7 @@ b3 =
 		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>
+const b4 = <div>
 	Text <a data-very-long-prop-breakline-rome-playground data-other>
 	some link
 </a>{' '}

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

@@ -98,17 +98,7 @@ b3 =
 		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>
+const b4 = <div>
 	Text <a data-very-long-prop-breakline-rome-playground data-other>
 	some link
 </a>{' '}
@@ -368,7 +358,10 @@ c2 = (
 // this group should fit one line jsx whitespaces are hidden
 b = (
 	<div>
-		<a></a> <a></a> 1
+		<a></a>
+
+		<a></a>
+		1
 	</div>
 );
 
@@ -381,14 +374,17 @@ b1 = (
 12312
 			`}
 		</a>{" "}
-		<a></a> 1
+
+		<a></a>
+		1
 	</div>
 );
 
 // this group fit one line and hide jsx whitespace
 b2 = (
 	<>
-		<a>123</a> 1
+		<a>123</a>
+		1
 	</>
 );
 
@@ -403,35 +399,7 @@ b3 = (
 	</>
 );
 
-// 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 = (
+const b4 = (
 	<div>
 		Text <a data-very-long-prop-breakline-rome-playground data-other>
 			some link
@@ -649,9 +617,9 @@ const breadcrumbItems = [
     2: <div tooltip="A very long tooltip text that would otherwise make the attribute break onto the same line but it is not because of the single string layout"></div>;
     7: 		tooltip="A very long tooltip text that would otherwise make the attribute break
    14: 	<ASuperLongComponentNameThatWouldBreakButDoesntSinceTheComponent></ASuperLongComponentNameThatWouldBreakButDoesntSinceTheComponent>
-  201: 							"ui-monospace,SFMono-Regular,SF Mono,Consolas,Liberation Mono,Menlo,monospace",
-  222: 							"ui-monospace,SFMono-Regular,SF Mono,Consolas,Liberation Mono,Menlo,monospace",
-  235: 							"ui-monospace,SFMono-Regular,SF Mono,Consolas,Liberation Mono,Menlo,monospace",
-  260: 				<pre className="h-screen overflow-y-scroll whitespace-pre-wrap text-red-500 text-xs">
-  309: 		Uncle Boonmee Who Can Recall His Past Lives dir. Apichatpong Weerasethakul{" "}
+  179: 							"ui-monospace,SFMono-Regular,SF Mono,Consolas,Liberation Mono,Menlo,monospace",
+  200: 							"ui-monospace,SFMono-Regular,SF Mono,Consolas,Liberation Mono,Menlo,monospace",
+  213: 							"ui-monospace,SFMono-Regular,SF Mono,Consolas,Liberation Mono,Menlo,monospace",
+  238: 				<pre className="h-screen overflow-y-scroll whitespace-pre-wrap text-red-500 text-xs">
+  287: 		Uncle Boonmee Who Can Recall His Past Lives dir. Apichatpong Weerasethakul{" "}
 

crates/rome_js_formatter/tests/specs/prettier/jsx/significant-space/test.js.snap

@@ -89,25 +89,18 @@ not_broken_begin =
 ```diff
 --- Prettier
 +++ Rome
-@@ -12,7 +12,7 @@
+@@ -12,8 +12,7 @@
  
  before_break1 = (
    <span>
 -    <span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar />{" "}
-+    <span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar />
-     foo
-   </span>
- );
-@@ -21,15 +21,16 @@
-   <span>
-     <span
-       barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar
--    />{" "}
-+    />
-     foo
+-    foo
++    <span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar /> foo
    </span>
  );
  
+@@ -28,8 +27,9 @@
+ 
  after_break = (
    <span>
 -    foo{" "}
@@ -118,43 +111,15 @@ not_broken_begin =
    </span>
  );
  
-@@ -79,7 +80,8 @@
- 
- regression_not_transformed_2 = (
-   <span>
--    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />{" "}
-+    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
-+    {" "}
-   </span>
- );
- 
-@@ -92,13 +94,15 @@
- 
- similar_2 = (
-   <span>
--    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />{" "}
-+    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
-+    {" "}
-   </span>
- );
- 
- similar_3 = (
-   <span>
--    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" /> <Icon icon="" />
-+    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
-+    <Icon icon="" />
-   </span>
- );
- 
-@@ -111,7 +115,8 @@
+@@ -111,7 +111,8 @@
  
  not_broken_begin = (
    <div>
 -    <br /> long text long text long text long text long text long text long text
 -    long text<link>url</link> long text long text
-+    <br />
-+    long text long text long text long text long text long text long text long
-+    text<link>url</link> long text long text
++    <br /> long text long text long text long text long text long text long text long text<link>
++      url
++    </link> long text long text
    </div>
  );
 ```
@@ -176,16 +141,15 @@ before = (
 
 before_break1 = (
   <span>
-    <span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar />
-    foo
+    <span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar /> foo
   </span>
 );
 
 before_break2 = (
   <span>
     <span
       barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar
-    />
+    />{" "}
     foo
   </span>
 );
@@ -244,8 +208,7 @@ regression_not_transformed_1 = (
 
 regression_not_transformed_2 = (
   <span>
-    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
-    {" "}
+    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />{" "}
   </span>
 );
 
@@ -258,15 +221,13 @@ similar_1 = (
 
 similar_2 = (
   <span>
-    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
-    {" "}
+    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />{" "}
   </span>
 );
 
 similar_3 = (
   <span>
-    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
-    <Icon icon="" />
+    <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" /> <Icon icon="" />
   </span>
 );
 
@@ -279,12 +240,18 @@ not_broken_end = (
 
 not_broken_begin = (
   <div>
-    <br />
-    long text long text long text long text long text long text long text long
-    text<link>url</link> long text long text
+    <br /> long text long text long text long text long text long text long text long text<link>
+      url
+    </link> long text long text
   </div>
 );
 ```
 
 
+# Lines exceeding max width of 80 characters
+```
+   15:     <span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar /> foo
+   95:     <Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />{" "}
+  114:     <br /> long text long text long text long text long text long text long text long text<link>
+```