Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(js_formatter): fix JSX text wrapping and empty line handling #1075

Merged
merged 1 commit into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 29 additions & 7 deletions crates/biome_js_formatter/src/jsx/lists/child_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl FormatJsxChildList {
is_soft_line_break: !matches!(
next_child,
AnyJsxChild::JsxSelfClosingElement(_)
) || word.is_ascii_punctuation(),
) || word.is_single_character(),
}),

Some(JsxChild::Newline | JsxChild::Whitespace | JsxChild::EmptyLine) => {
Expand Down Expand Up @@ -166,7 +166,7 @@ impl FormatJsxChildList {
// A new line between some JSX text and an element
JsxChild::Newline => {
let is_soft_break = {
// Here we handle the case when we have a newline between an ascii punctuation word and a jsx element
// Here we handle the case when we have a newline between a single-character word and a jsx element
// We need to use the previous and the next element
// [JsxChild::Word, JsxChild::Newline, JsxChild::NonText]
// ```
Expand All @@ -180,9 +180,9 @@ impl FormatJsxChildList {
children_iter.peek(),
Some(JsxChild::NonText(AnyJsxChild::JsxSelfClosingElement(_)))
);
!is_next_element_self_closing && word.is_ascii_punctuation()
!is_next_element_self_closing && word.is_single_character()
}
// Here we handle the case when we have an ascii punctuation word between a new line and a jsx element
// Here we handle the case when we have a single-character word between a new line and a jsx element
// Here we need to look ahead two elements
// [JsxChild::Newline, JsxChild::Word, JsxChild::NonText]
// ```
Expand All @@ -207,7 +207,7 @@ impl FormatJsxChildList {

!has_new_line_and_self_closing
&& !is_next_next_element_self_closing
&& next_word.is_ascii_punctuation()
&& next_word.is_single_character()
} else {
false
}
Expand All @@ -225,7 +225,29 @@ impl FormatJsxChildList {
JsxChild::EmptyLine => {
child_breaks = true;

multiline.write_separator(&empty_line(), f);
// Additional empty lines are not preserved when any of
// the children are a meaningful text node.
//
// <>
// <div>First</div>
//
// <div>Second</div>
//
// Third
// </>
//
// Becomes:
//
// <>
// <div>First</div>
// <div>Second</div>
// Third
// </>
if children_meta.meaningful_text {
multiline.write_separator(&hard_line_break(), f);
} else {
multiline.write_separator(&empty_line(), f);
}
}

// Any child that isn't text
Expand All @@ -242,7 +264,7 @@ impl FormatJsxChildList {
// adefg
// ```
if matches!(non_text, AnyJsxChild::JsxSelfClosingElement(_))
&& !word.is_ascii_punctuation()
&& !word.is_single_character()
{
Some(LineMode::Hard)
} else {
Expand Down
7 changes: 1 addition & 6 deletions crates/biome_js_formatter/src/utils/jsx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,13 +413,8 @@ impl JsxWord {
}
}

pub(crate) fn is_ascii_punctuation(&self) -> bool {
pub(crate) fn is_single_character(&self) -> bool {
self.text.chars().count() == 1
&& self
.text
.chars()
.next()
.map_or(false, |char| char.is_ascii_punctuation())
}
}

Expand Down
102 changes: 102 additions & 0 deletions crates/biome_js_formatter/tests/specs/jsx/text_children.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Short, textual children can collapse onto the same line as self-closing
// elements if they start or end with a single character word or a space.
// Elements containing children will also remove all empty lines within them.
// This tests various permutations of that.

<>
a<div />b
</>;
<>
a<div />bb
</>;
<>
aa<div />b
</>;
<>
aa<div />bb
</>;

// As long as the first/last word of the text has a single character,
// it can stay on the same line.
<>
a b<div />c
</>;
<>
a bb<div />c
</>;
<>
aa b<div />c
</>;
<>
aa bb<div />c
</>;
<>
a<div />b c
</>;
<>
a<div />b ccc
</>;
<>
a<div />bb cc
</>;
<>
aa<div />b c
</>;
<>
aa<div />b ccc
</>;
<>
aa<div />bb cc
</>;
<>
longword doesntmatter a<div />b
</>;
<>
a<div />b longword doesntmatter
</>;


// Any character counts
<>
1<div />b
</>;
<>
11<div />b
</>;
<>
ม<div />b
</>;
<>
มม<div />b
</>;
<>
!<div />b
</>;
<>
!!<div />b
</>;

// Spaces also count
<>
a <div />b
</>;
<>
aa <div />b
</>;

// Blank lines aren't kept if the children contain meaningful text
<>
line


2
</>;
<>
first


<div>second</div>


<div>third</div>
</>;