Skip to content

Commit 4262c98

Browse files
denbezrukovdenbezrukov
committed
fix(rome_js_formatter): in JSX, some spaces are removed rome#3211 
1 parent 73defc1 commit 4262c98

File tree

5 files changed

+329
-30
lines changed

5 files changed

+329
-30
lines changed

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

+9-3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::JsFormatter;
77
use rome_formatter::format_element::tag::Tag;
88
use rome_formatter::{format_args, write, CstFormatContext, FormatRuleWithOptions, VecBuffer};
99
use rome_js_syntax::{JsxAnyChild, JsxChildList};
10+
use rome_rowan::TextSize;
1011
use std::cell::RefCell;
1112

1213
#[derive(Debug, Clone, Default)]
@@ -174,7 +175,7 @@ impl FormatJsxChildList {
174175
// Any child that isn't text
175176
JsxChild::NonText(non_text) => {
176177
let line_mode = match children_iter.peek() {
177-
Some(JsxChild::Newline | JsxChild::Word(_) | JsxChild::Whitespace) => {
178+
Some(JsxChild::Word(word)) => {
178179
// Break if the current or next element is a self closing element
179180
// ```javascript
180181
// <pre className="h-screen overflow-y-scroll" />adefg
@@ -184,16 +185,21 @@ impl FormatJsxChildList {
184185
// <pre className="h-screen overflow-y-scroll" />
185186
// adefg
186187
// ```
187-
if matches!(non_text, JsxAnyChild::JsxSelfClosingElement(_)) {
188+
if matches!(non_text, JsxAnyChild::JsxSelfClosingElement(_))
189+
&& word.len() != TextSize::from(1)
190+
{
188191
Some(LineMode::Hard)
189192
} else {
190193
Some(LineMode::Soft)
191194
}
192195
}
193196

194197
// Add a hard line break if what comes after the element is not a text or is all whitespace
195-
Some(_) => Some(LineMode::Hard),
198+
Some(JsxChild::NonText(_)) => Some(LineMode::Hard),
196199

200+
Some(JsxChild::Newline | JsxChild::Whitespace | JsxChild::EmptyLine) => {
201+
None
202+
}
197203
// Don't insert trailing line breaks
198204
None => None,
199205
};

‎crates/rome_js_formatter/src/lib.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -861,10 +861,8 @@ function() {
861861
// use this test check if your snippet prints as you wish, without using a snapshot
862862
fn quick_test() {
863863
let src = r#"
864-
new Test()
865-
.test()
866-
.test([, 0])
867-
.test();
864+
const a = <><b/>a{' '}{" "}{" "}{" "}c{" "}{" "}{" "}{" "}{" "}{" "}</>;
865+
868866
869867
"#;
870868
let syntax = SourceType::jsx();

‎crates/rome_js_formatter/src/utils/jsx.rs

+100-18
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ impl Format<JsFormatContext> for JsxSpace {
168168
write![
169169
formatter,
170170
[
171-
if_group_breaks(&format_args![JsxRawSpace, hard_line_break()]),
171+
if_group_breaks(&format_args![JsxRawSpace, soft_line_break()]),
172172
if_group_fits_on_line(&space())
173173
]
174174
]
@@ -223,7 +223,7 @@ pub(crate) fn jsx_split_children<I>(
223223
where
224224
I: IntoIterator<Item = JsxAnyChild>,
225225
{
226-
let mut result = Vec::new();
226+
let mut builder = JsxSplitChildrenBuilder::default();
227227

228228
for child in children.into_iter() {
229229
match child {
@@ -253,15 +253,15 @@ where
253253
// </div>
254254
// ```
255255
if newlines > 1 {
256-
result.push(JsxChild::EmptyLine);
256+
builder.entry(JsxChild::EmptyLine);
257257
}
258258

259259
continue;
260260
}
261261

262-
result.push(JsxChild::Newline)
263-
} else if !matches!(result.last(), Some(JsxChild::Whitespace)) {
264-
result.push(JsxChild::Whitespace)
262+
builder.entry(JsxChild::Newline)
263+
} else {
264+
builder.entry(JsxChild::Whitespace)
265265
}
266266
}
267267
_ => unreachable!(),
@@ -274,15 +274,15 @@ where
274274
// Only handle trailing whitespace. Words must always be joined by new lines
275275
if chunks.peek().is_none() {
276276
if whitespace.contains('\n') {
277-
result.push(JsxChild::Newline);
277+
builder.entry(JsxChild::Newline);
278278
} else {
279-
result.push(JsxChild::Whitespace)
279+
builder.entry(JsxChild::Whitespace)
280280
}
281281
}
282282
}
283283

284284
(relative_start, JsxTextChunk::Word(word)) => {
285-
result.push(JsxChild::Word(JsxWord {
285+
builder.entry(JsxChild::Word(JsxWord {
286286
text: value_token
287287
.token_text()
288288
.slice(TextRange::at(relative_start, word.text_len())),
@@ -295,23 +295,61 @@ where
295295

296296
JsxAnyChild::JsxExpressionChild(child) => {
297297
if is_whitespace_jsx_expression(&child, comments) {
298-
match result.last() {
299-
Some(JsxChild::Whitespace) => {
300-
// Ignore
301-
}
302-
_ => result.push(JsxChild::Whitespace),
303-
}
298+
builder.entry(JsxChild::Whitespace)
304299
} else {
305-
result.push(JsxChild::NonText(child.into()))
300+
builder.entry(JsxChild::NonText(child.into()))
306301
}
307302
}
308303
child => {
309-
result.push(JsxChild::NonText(child));
304+
builder.entry(JsxChild::NonText(child));
310305
}
311306
}
312307
}
313308

314-
Ok(result)
309+
Ok(builder.finish())
310+
}
311+
312+
#[derive(Debug, Default)]
313+
struct JsxSplitChildrenBuilder {
314+
pending: Option<JsxChild>,
315+
buffer: Vec<JsxChild>,
316+
}
317+
318+
impl JsxSplitChildrenBuilder {
319+
fn is_last_whitespace(&self) -> bool {
320+
matches!(self.buffer.last(), Some(JsxChild::Whitespace))
321+
}
322+
323+
fn entry(&mut self, child: JsxChild) {
324+
if let Some(pending) = self.pending.take() {
325+
if matches!(
326+
pending,
327+
JsxChild::EmptyLine | JsxChild::Newline | JsxChild::Whitespace
328+
) {
329+
if !matches!(child, JsxChild::Whitespace) && !self.is_last_whitespace() {
330+
self.buffer.push(pending)
331+
}
332+
} else {
333+
self.buffer.push(pending);
334+
}
335+
}
336+
337+
self.pending = Some(child);
338+
}
339+
340+
fn finish(mut self) -> Vec<JsxChild> {
341+
if let Some(pending) = self.pending.take() {
342+
if matches!(pending, JsxChild::Whitespace) {
343+
if !self.is_last_whitespace() {
344+
self.buffer.push(pending)
345+
}
346+
} else {
347+
self.buffer.push(pending);
348+
}
349+
}
350+
351+
self.buffer
352+
}
315353
}
316354

317355
#[derive(Debug, Clone, Eq, PartialEq)]
@@ -378,6 +416,12 @@ pub(crate) struct JsxWord {
378416
source_position: TextSize,
379417
}
380418

419+
impl JsxWord {
420+
pub fn len(&self) -> TextSize {
421+
self.text.len()
422+
}
423+
}
424+
381425
impl Format<JsFormatContext> for JsxWord {
382426
fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> FormatResult<()> {
383427
f.write_element(FormatElement::Text(Text::SyntaxTokenTextSlice {
@@ -645,6 +689,44 @@ mod tests {
645689
assert_eq!(children[3], JsxChild::Whitespace);
646690
}
647691

692+
#[test]
693+
fn split_children_remove_in_row_jsx_whitespaces() {
694+
let child_list = parse_jsx_children(r#"a{' '}{' '}{' '}c{" "}{' '}{" "}"#);
695+
696+
let children = jsx_split_children(&child_list).unwrap();
697+
698+
assert_eq!(
699+
4,
700+
children.len(),
701+
"Expected to contain four elements. Actual:\n{children:#?} "
702+
);
703+
assert_word(&children[0], "a");
704+
assert_eq!(children[1], JsxChild::Whitespace);
705+
assert_word(&children[2], "c");
706+
assert_eq!(children[3], JsxChild::Whitespace);
707+
}
708+
709+
#[test]
710+
fn split_children_remove_new_line_before_jsx_whitespaces() {
711+
let child_list = parse_jsx_children(
712+
r#"a
713+
{' '}c{" "}
714+
"#,
715+
);
716+
717+
let children = jsx_split_children(&child_list).unwrap();
718+
719+
assert_eq!(
720+
4,
721+
children.len(),
722+
"Expected to contain four elements. Actual:\n{children:#?} "
723+
);
724+
assert_word(&children[0], "a");
725+
assert_eq!(children[1], JsxChild::Whitespace);
726+
assert_word(&children[2], "c");
727+
assert_eq!(children[3], JsxChild::Whitespace);
728+
}
729+
648730
fn assert_word(child: &JsxChild, text: &str) {
649731
match child {
650732
JsxChild::Word(word) => {

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

+69
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,75 @@ c = <div>a{' '}{' '}{' '}{' '}{' '}{' '}{' '}{' '}b{' '}{' '}{' '}{' '}{' '}{' '
4141

4242
c2 = <div>a{' '}{' '}{' '}{' '}{' '}{' '}{' '}{' '}<div></div>content{' '}{' '}{' '}{' '}{' '}{' '}</div>;
4343

44+
// this group should fit one line jsx whitespaces are hidden
45+
b =
46+
<div>
47+
<a></a>
48+
49+
{' '}
50+
51+
<a></a>
52+
53+
{' '}
54+
1
55+
</div>;
56+
57+
// this group should break first tag and show only first jsx whitespace
58+
b1 =
59+
<div>
60+
<a>
61+
{`
62+
12312
63+
12312
64+
`}
65+
</a>
66+
67+
{' '}
68+
69+
<a></a>
70+
71+
{' '}
72+
1
73+
</div>;
74+
75+
// this group fit one line and hide jsx whitespace
76+
b2 =
77+
<>
78+
<a>123
79+
</a>
80+
81+
{' '}
82+
1
83+
</>;
84+
85+
// this group break group and show jsx whitespace
86+
b3 =
87+
<>
88+
<a>{`
89+
123`}
90+
</a>
91+
92+
{' '}
93+
1
94+
</>;
95+
96+
// one length text should insert soft line break
97+
b4 = <><pre />a</>;
98+
// longer than one length text should insert hard line break
99+
b5 = <><pre />ab</>;
100+
101+
// one length text should insert soft line break show last jsx whitespace
102+
b6 = <><b/>a{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}</>;
103+
// longer than one length text should insert hard line break
104+
b7 = <><b/>longer{' '}bb{" "}cc{" "}dd{" "}ee{" "}{" "}{" "}{" "}{" "}{" "}</>;
105+
106+
const b8 = <div>
107+
Text <a data-very-long-prop-breakline-rome-playground data-other>
108+
some link
109+
</a>{' '}
110+
| some other text,{' '}
111+
</div>;
112+
44113
<div><div></div><a> jumps over the lazy dog </a></div>;
45114

46115
const Essay = () => <div>The films of Wong Kar-Wai exemplify the synthesis of French New Wave cinema—specifically the unrelenting

0 commit comments

Comments
 (0)