Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_formatter): in JSX, some spaces are removed #3211
Browse files Browse the repository at this point in the history
  • Loading branch information
@denbezrukov
denbezrukov committed Sep 22, 2022
1 parent 3a007d4 commit 373e234
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 547 deletions.
3 changes: 1 addition & 2 deletions crates/rome_js_formatter/src/jsx/lists/child_list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand Down
57 changes: 24 additions & 33 deletions crates/rome_js_formatter/src/utils/jsx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
]
]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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())
}
}

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
12 changes: 1 addition & 11 deletions crates/rome_js_formatter/tests/specs/jsx/element.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>{' '}
Expand Down
64 changes: 16 additions & 48 deletions crates/rome_js_formatter/tests/specs/jsx/element.jsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>{' '}
Expand Down Expand Up @@ -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>
);

Expand All @@ -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
</>
);

Expand All @@ -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
Expand Down Expand Up @@ -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{" "}

79 changes: 23 additions & 56 deletions crates/rome_js_formatter/tests/specs/prettier/jsx/significant-space/test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{" "}
Expand All @@ -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>
);
```
Expand All @@ -176,16 +141,15 @@ before = (

before_break1 = (
<span>
<span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar />
foo
<span barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar /> foo
</span>
);

before_break2 = (
<span>
<span
barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar
/>
/>{" "}
foo
</span>
);
Expand Down Expand Up @@ -244,8 +208,7 @@ regression_not_transformed_1 = (

regression_not_transformed_2 = (
<span>
<Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
{" "}
<Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />{" "}
</span>
);

Expand All @@ -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>
);

Expand All @@ -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>
```

Loading

0 comments on commit 373e234

Please sign in to comment.