From 373e234404c2e5dd045323f49de0883e156125c2 Mon Sep 17 00:00:00 2001
From: Denis Bezrukov <6227442+denbezrukov@users.noreply.github.com>
Date: Thu, 22 Sep 2022 19:28:46 +0300
Subject: [PATCH] fix(rome_js_formatter): in JSX, some spaces are removed #3211
---
.../src/jsx/lists/child_list.rs | 3 +-
crates/rome_js_formatter/src/utils/jsx.rs | 57 ++--
.../tests/specs/jsx/element.jsx | 12 +-
.../tests/specs/jsx/element.jsx.snap | 64 +---
.../jsx/significant-space/test.js.snap | 79 ++---
.../jsx/stateless-arrow-fn/test.js.snap | 300 ------------------
.../specs/prettier/jsx/text-wrap/test.js.snap | 161 ++++------
7 files changed, 129 insertions(+), 547 deletions(-)
delete mode 100644 crates/rome_js_formatter/tests/specs/prettier/jsx/stateless-arrow-fn/test.js.snap
diff --git a/crates/rome_js_formatter/src/jsx/lists/child_list.rs b/crates/rome_js_formatter/src/jsx/lists/child_list.rs
index a8ff3a1aad51..d8433811b85a 100644
--- a/crates/rome_js_formatter/src/jsx/lists/child_list.rs
+++ b/crates/rome_js_formatter/src/jsx/lists/child_list.rs
@@ -6,7 +6,6 @@ use crate::utils::jsx::{
use crate::JsFormatter;
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)]
@@ -185,7 +184,7 @@ impl FormatJsxChildList {
// adefg
// ```
if matches!(non_text, JsxAnyChild::JsxSelfClosingElement(_))
- && word.len() != TextSize::from(1)
+ && !word.is_ascii_punctuation()
{
Some(LineMode::Hard)
} else {
diff --git a/crates/rome_js_formatter/src/utils/jsx.rs b/crates/rome_js_formatter/src/utils/jsx.rs
index 4af72989e606..f719223b458c 100644
--- a/crates/rome_js_formatter/src/utils/jsx.rs
+++ b/crates/rome_js_formatter/src/utils/jsx.rs
@@ -168,7 +168,7 @@ impl Format 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(
where
I: IntoIterator,
{
- 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,
buffer: Vec,
}
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 {
- 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 {
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,
diff --git a/crates/rome_js_formatter/tests/specs/jsx/element.jsx b/crates/rome_js_formatter/tests/specs/jsx/element.jsx
index b22afc059328..6ddad1660e11 100644
--- a/crates/rome_js_formatter/tests/specs/jsx/element.jsx
+++ b/crates/rome_js_formatter/tests/specs/jsx/element.jsx
@@ -93,17 +93,7 @@ b3 =
1
>;
-// one length text should insert soft line break
-b4 = <>a>;
-// longer than one length text should insert hard line break
-b5 = <>ab>;
-
-// one length text should insert soft line break show last jsx whitespace
-b6 = <>a{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}>;
-// longer than one length text should insert hard line break
-b7 = <>longer{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}>;
-
-const b8 =
+const b4 =
Text
some link
{' '}
diff --git a/crates/rome_js_formatter/tests/specs/jsx/element.jsx.snap b/crates/rome_js_formatter/tests/specs/jsx/element.jsx.snap
index 66eb6a9be4f6..c7e24c34ea62 100644
--- a/crates/rome_js_formatter/tests/specs/jsx/element.jsx.snap
+++ b/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 = <>a>;
-// longer than one length text should insert hard line break
-b5 = <>ab>;
-
-// one length text should insert soft line break show last jsx whitespace
-b6 = <>a{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}>;
-// longer than one length text should insert hard line break
-b7 = <>longer{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}>;
-
-const b8 =
+const b4 =
Text
some link
{' '}
@@ -368,7 +358,10 @@ c2 = (
// this group should fit one line jsx whitespaces are hidden
b = (
);
// this group fit one line and hide jsx whitespace
b2 = (
<>
- 123 1
+ 123
+ 1
>
);
@@ -403,35 +399,7 @@ b3 = (
>
);
-// one length text should insert soft line break
-b4 = (
- <>
- a
- >
-);
-// longer than one length text should insert hard line break
-b5 = (
- <>
-
- ab
- >
-);
-
-// one length text should insert soft line break show last jsx whitespace
-b6 = (
- <>
- a bb cc dd ee{" "}
- >
-);
-// longer than one length text should insert hard line break
-b7 = (
- <>
-
- longer bb cc dd ee{" "}
- >
-);
-
-const b8 = (
+const b4 = (
@@ -514,8 +517,7 @@ let myDiv = ReactTestUtils.renderIntoDocument(
- {stuff} {stuff} {stuff} after {stuff} after
+ before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {
+ stuff
-+ }
-+ {stuff} {stuff} after {stuff} after
++ } {stuff} {stuff} after {stuff} after
);
@@ -525,12 +527,22 @@ let myDiv = ReactTestUtils.renderIntoDocument(
- school directors.
+ Please state your name and occupation for the board of
+ school
-+
++ {" "}
+ directors.
);
-@@ -101,9 +108,10 @@
+@@ -89,8 +96,7 @@
+ .
+
+
+- Note:last_modified and schema record
+- metadata are omitted for easier review.
++ Note:last_modified and schema record metadata are omitted for easier review.
+
@@ -580,11 +592,7 @@ let myDiv = ReactTestUtils.renderIntoDocument(
{
"Enough text to make this element wrap on to multiple lines when formatting"
}
--
@@ -594,26 +602,19 @@ let myDiv = ReactTestUtils.renderIntoDocument(
{
"Enough text to make this element wrap on to multiple lines when formatting"
}
--
- Please state your name and occupation for the board of
- directors.
-+ Please state your
-+ {" "}name
-+ and
-+ {" "}occupation
++ Please state your name
++ and occupation
+ for the board of directors.
@@ -659,32 +653,22 @@ let myDiv = ReactTestUtils.renderIntoDocument(
- {this.props.type}{" "}
+ texty text text text text text text text text text text text {
+ this.props.type
-+ }
-+ {" "}
++ }{" "}
- text text text text text text text text text text text {
- this.props.type
- }{" "}
-+ text text text text text text text text text text text {this.props.type}
-+ {" "}
++ text text text text text text text text text text text {this.props.type}{" "}
);
-@@ -372,14 +387,14 @@
-
-
- {variable}
-- {" "}
-+
- ({variable})
-
- );
+@@ -379,7 +388,7 @@
x = (
@@ -693,19 +677,18 @@ let myDiv = ReactTestUtils.renderIntoDocument(
HRS
);
@@ -824,8 +797,9 @@ x = (
// Wrapping tags
x = (
-
- f
+
+ f
+
);
@@ -852,8 +826,7 @@ x = (
before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {
stuff
- }
- {stuff} {stuff} after {stuff} after
+ } {stuff} {stuff} after {stuff} after
);
@@ -861,7 +834,7 @@ x = (
Please state your name and occupation for the board of
school
-
+ {" "}
directors.
);
@@ -877,8 +850,7 @@ function DiffOverview(props) {
.
- Note:last_modified and schema record
- metadata are omitted for easier review.
+ Note:last_modified and schema record metadata are omitted for easier review.
@@ -891,7 +863,7 @@ x = (
Starting at minute {graphActivity.startTime}, running for {
graphActivity.length
- }
+ }{" "}
to minute {graphActivity.startTime + graphActivity.length}
@@ -1004,7 +976,7 @@ jsx_around_multiline_element = (
{
"Enough text to make this element wrap on to multiple lines when formatting"
}
-
+ {" "}
After
);
@@ -1015,7 +987,7 @@ jsx_around_multiline_element_second_pass = (
{
"Enough text to make this element wrap on to multiple lines when formatting"
}
-
+ {" "}
After
);
@@ -1036,10 +1008,8 @@ x = (
const Abc = () => {
return (
- Please state your
- {" "}name
- and
- {" "}occupation
+ Please state your name
+ and occupation
for the board of directors.
- text text text text text text text text text text text {this.props.type}
- {" "}
+ text text text text text text text text text text text {this.props.type}{" "}
,
);
@@ -1382,9 +1347,11 @@ let myDiv = ReactTestUtils.renderIntoDocument(
# Lines exceeding max width of 80 characters
```
- 72: before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {
- 122:
- 224: "Enough text to make this element wrap on to multiple lines when formatting"
- 235: "Enough text to make this element wrap on to multiple lines when formatting"
+ 73: before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {
+ 99: Note:last_modified and schema record metadata are omitted for easier review.
+ 121:
+ 223: "Enough text to make this element wrap on to multiple lines when formatting"
+ 234: "Enough text to make this element wrap on to multiple lines when formatting"
+ 376: text text text text text text text text text text text {this.props.type}{" "}
```