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 9987c4b389f..f0f334b789c 100644 --- a/crates/rome_js_formatter/src/jsx/lists/child_list.rs +++ b/crates/rome_js_formatter/src/jsx/lists/child_list.rs @@ -1,7 +1,7 @@ use crate::prelude::*; use crate::utils::jsx::{ is_meaningful_jsx_text, is_whitespace_jsx_expression, jsx_split_children, JsxChild, - JsxRawSpace, JsxSpace, + JsxChildrenIterator, JsxRawSpace, JsxSpace, }; use crate::JsFormatter; use rome_formatter::format_element::tag::{GroupMode, Tag}; @@ -72,7 +72,6 @@ impl FormatJsxChildList { let mut flat = FlatBuilder::new(); let mut multiline = MultilineBuilder::new(multiline_layout); - let mut last: Option = None; let mut force_multiline = layout.is_multiline(); let mut children = jsx_split_children(list, f.context().comments())?; @@ -82,7 +81,8 @@ impl FormatJsxChildList { children.pop(); } - let mut children_iter = children.into_iter().peekable(); + let mut last: Option<&JsxChild> = None; + let mut children_iter = JsxChildrenIterator::new(children.iter()); // Trim leading new lines if let Some(JsxChild::Newline | JsxChild::EmptyLine) = children_iter.peek() { @@ -102,11 +102,11 @@ impl FormatJsxChildList { } // Last word or last word before an element without any whitespace in between - Some(JsxChild::NonText(child)) => Some(WordSeparator::EndOfText { - is_next_self_closing: matches!( - child, + Some(JsxChild::NonText(next_child)) => Some(WordSeparator::EndOfText { + is_soft_line_break: !matches!( + next_child, JsxAnyChild::JsxSelfClosingElement(_) - ), + ) || word.is_ascii_punctuation(), }), Some(JsxChild::Newline | JsxChild::Whitespace | JsxChild::EmptyLine) => { @@ -165,9 +165,50 @@ impl FormatJsxChildList { // A new line between some JSX text and an element JsxChild::Newline => { - child_breaks = true; + let is_soft_break = { + // Here we handle the case when we have a newline between an ascii punctuation word and a jsx element + // We need to use the previous and the next element + // [JsxChild::Word, JsxChild::Newline, JsxChild::NonText] + // ``` + //
+ //
First
, + //
Second
+ //
+ // ``` + if let Some(JsxChild::Word(word)) = last { + let is_next_element_self_closing = matches!( + children_iter.peek(), + Some(JsxChild::NonText(JsxAnyChild::JsxSelfClosingElement(_))) + ); + !is_next_element_self_closing && word.is_ascii_punctuation() + } + // Here we handle the case when we have an ascii punctuation word between a new line and a jsx element + // Here we need to look ahead two elements + // [JsxChild::Newline, JsxChild::Word, JsxChild::NonText] + // ``` + //
+ //
First
+ // ,
Second
+ //
+ // ``` + else if let Some(JsxChild::Word(next_word)) = children_iter.peek() { + let is_next_next_element_self_closing = matches!( + children_iter.peek_next(), + Some(JsxChild::NonText(JsxAnyChild::JsxSelfClosingElement(_))) + ); + + !is_next_next_element_self_closing && next_word.is_ascii_punctuation() + } else { + false + } + }; - multiline.write_separator(&hard_line_break(), f); + if is_soft_break { + multiline.write_separator(&soft_line_break(), f); + } else { + child_breaks = true; + multiline.write_separator(&hard_line_break(), f); + } } // An empty line between some JSX text and an element @@ -406,7 +447,8 @@ enum WordSeparator { ///
a{expression}
// last element before expression /// ``` /// - /// Creates a soft line break EXCEPT if the next element is a self closing element, which results in a hard line break: + /// Creates a soft line break EXCEPT if the next element is a self closing element + /// or the previous word was an ascii punctuation, which results in a hard line break: /// /// ```javascript /// a =
ab
; @@ -420,10 +462,7 @@ enum WordSeparator { /// /// ); /// ``` - EndOfText { - /// `true` if the next element is a [JsxSelfClosingElement] - is_next_self_closing: bool, - }, + EndOfText { is_soft_line_break: bool }, } impl WordSeparator { @@ -432,7 +471,7 @@ impl WordSeparator { matches!( self, WordSeparator::EndOfText { - is_next_self_closing: true + is_soft_line_break: false, } ) } @@ -442,9 +481,10 @@ impl Format for WordSeparator { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { match self { WordSeparator::BetweenWords => soft_line_break_or_space().fmt(f), - WordSeparator::EndOfText { - is_next_self_closing: self_closing, - } => { + WordSeparator::EndOfText { is_soft_line_break } => { + if *is_soft_line_break { + soft_line_break().fmt(f) + } // ```javascript //
ab
// ``` @@ -456,12 +496,8 @@ impl Format for WordSeparator { //
// // ``` - if *self_closing { - hard_line_break().fmt(f) - } - // Try to fit everything else on a single line else { - soft_line_break().fmt(f) + hard_line_break().fmt(f) } } } diff --git a/crates/rome_js_formatter/src/utils/jsx.rs b/crates/rome_js_formatter/src/utils/jsx.rs index c71ed4b9e42..8d0a7cbfeeb 100644 --- a/crates/rome_js_formatter/src/utils/jsx.rs +++ b/crates/rome_js_formatter/src/utils/jsx.rs @@ -282,12 +282,12 @@ where } (relative_start, JsxTextChunk::Word(word)) => { - builder.entry(JsxChild::Word(JsxWord { - text: value_token - .token_text() - .slice(TextRange::at(relative_start, word.text_len())), - source_position: value_token.text_range().start() + relative_start, - })); + let text = value_token + .token_text() + .slice(TextRange::at(relative_start, word.text_len())); + let source_position = value_token.text_range().start() + relative_start; + + builder.entry(JsxChild::Word(JsxWord::new(text, source_position))); } } } @@ -406,7 +406,14 @@ pub(crate) struct JsxWord { } impl JsxWord { - pub fn is_ascii_punctuation(&self) -> bool { + fn new(text: SyntaxTokenText, source_position: TextSize) -> Self { + JsxWord { + text, + source_position, + } + } + + pub(crate) fn is_ascii_punctuation(&self) -> bool { self.text.chars().count() == 1 && self .text @@ -487,15 +494,101 @@ impl<'a> Iterator for JsxSplitChunksIterator<'a> { impl FusedIterator for JsxSplitChunksIterator<'_> {} +/// An iterator adaptor that allows a lookahead of two tokens +/// +/// # Examples +/// ``` +/// use rome_js_formatter::utils::jsx::JsxChildrenIterator; +/// +/// let buffer = vec![1, 2, 3, 4]; +/// +/// let mut iter = JsxChildrenIterator::new(buffer.iter()); +/// +/// assert_eq!(iter.peek(), Some(&&1)); +/// assert_eq!(iter.peek_next(), Some(&&2)); +/// assert_eq!(iter.next(), Some(&1)); +/// assert_eq!(iter.next(), Some(&2)); +/// ``` +#[derive(Clone, Debug)] +pub struct JsxChildrenIterator { + iter: I, + + peeked: Option>, + peeked_next: Option>, +} + +impl JsxChildrenIterator { + pub fn new(iter: I) -> Self { + Self { + iter, + peeked: None, + peeked_next: None, + } + } + + pub fn peek(&mut self) -> Option<&I::Item> { + let iter = &mut self.iter; + self.peeked.get_or_insert_with(|| iter.next()).as_ref() + } + + pub fn peek_next(&mut self) -> Option<&I::Item> { + let iter = &mut self.iter; + let peeked = &mut self.peeked; + + self.peeked_next + .get_or_insert_with(|| { + peeked.get_or_insert_with(|| iter.next()); + iter.next() + }) + .as_ref() + } +} + +impl Iterator for JsxChildrenIterator { + type Item = I::Item; + + fn next(&mut self) -> Option { + match self.peeked.take() { + Some(peeked) => { + self.peeked = self.peeked_next.take(); + peeked + } + None => self.iter.next(), + } + } +} + #[cfg(test)] mod tests { - use crate::utils::jsx::{jsx_split_children, JsxChild, JsxSplitChunksIterator, JsxTextChunk}; + use crate::utils::jsx::{ + jsx_split_children, JsxChild, JsxChildrenIterator, JsxSplitChunksIterator, JsxTextChunk, + }; use rome_diagnostics::location::FileId; use rome_formatter::comments::Comments; use rome_js_parser::parse; use rome_js_syntax::{JsxChildList, JsxText, SourceType}; use rome_rowan::{AstNode, TextSize}; + #[test] + fn jsx_children_iterator_test() { + let buffer = vec![1, 2, 3, 4]; + + let mut iter = JsxChildrenIterator::new(buffer.iter()); + + assert_eq!(iter.peek(), Some(&&1)); + assert_eq!(iter.peek(), Some(&&1)); + assert_eq!(iter.peek_next(), Some(&&2)); + assert_eq!(iter.peek_next(), Some(&&2)); + + assert_eq!(iter.next(), Some(&1)); + assert_eq!(iter.next(), Some(&2)); + + assert_eq!(iter.peek_next(), Some(&&4)); + assert_eq!(iter.peek_next(), Some(&&4)); + assert_eq!(iter.peek(), Some(&&3)); + assert_eq!(iter.peek(), Some(&&3)); + } + fn assert_jsx_text_chunks(text: &str, expected_chunks: Vec<(TextSize, JsxTextChunk)>) { let parse = parse( &std::format!("<>{text}"), diff --git a/crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx b/crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx new file mode 100644 index 00000000000..adc85ba1f8b --- /dev/null +++ b/crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx @@ -0,0 +1,50 @@ + + +x = ( +
+
First
,
Second
+
+); + +x = ( +
+
First
, +
Second
+
+); + +x = ( +
+
First
+ ,
Second
+
+); + +function Component() { + return ( + <> + text.
+ + ); +} + +let myDiv1 = ReactTestUtils.renderIntoDocument( + +
, + +); + + +let myDiv2 = ReactTestUtils.renderIntoDocument( + +
+ , + +); + +let myDiv3 = ReactTestUtils.renderIntoDocument( + +
, + + +); diff --git a/crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx.snap b/crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx.snap new file mode 100644 index 00000000000..d8348dbda2d --- /dev/null +++ b/crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx.snap @@ -0,0 +1,126 @@ +--- +source: crates/rome_js_formatter/tests/spec_test.rs +expression: new-lines.jsx +--- + +# Input + +```js + + +x = ( +
+
First
,
Second
+
+); + +x = ( +
+
First
, +
Second
+
+); + +x = ( +
+
First
+ ,
Second
+
+); + +function Component() { + return ( + <> + text.
+ + ); +} + +let myDiv1 = ReactTestUtils.renderIntoDocument( + +
, + +); + + +let myDiv2 = ReactTestUtils.renderIntoDocument( + +
+ , + +); + +let myDiv3 = ReactTestUtils.renderIntoDocument( + +
, + + +); + +``` + + +============================= + +# Outputs + +## Output 1 + +----- +Indent style: Tab +Line width: 80 +Quote style: Double Quotes +Quote properties: As needed +Trailing comma: All +Semicolons: Always +----- + +```js +x = ( +
+
First
,
Second
+
+); + +x = ( +
+
First
,
Second
+
+); + +x = ( +
+
First
,
Second
+
+); + +function Component() { + return ( + <> + text.
+ + ); +} + +let myDiv1 = ReactTestUtils.renderIntoDocument( + +
, + , +); + +let myDiv2 = ReactTestUtils.renderIntoDocument( + +
+ , + , +); + +let myDiv3 = ReactTestUtils.renderIntoDocument( + +
, + + , +); +``` + + diff --git a/crates/rome_js_formatter/tests/specs/prettier/jsx/fbt/test.js.snap b/crates/rome_js_formatter/tests/specs/prettier/jsx/fbt/test.js.snap index ca247636882..a1a4f417855 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/jsx/fbt/test.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/jsx/fbt/test.js.snap @@ -130,7 +130,45 @@ third = ( ```diff --- Prettier +++ Rome -@@ -80,15 +80,23 @@ +@@ -1,15 +1,12 @@ + x = ( + +- First, +- Second ++ First,Second + + ); + + x = ( + +- First +- , +- Second ++ First,Second + + ); + +@@ -62,33 +59,35 @@ + + x = ( + +- {hour} +- : +- {minute} +- : +- {second} ++ {hour}:{minute}:{second} + + ); + + x = ( + +- {hour}: +- {minute}: +- {second} ++ {hour}:{minute}:{second} + + ); first = ( @@ -165,16 +203,13 @@ third = ( ```js x = ( - First, - Second + First,Second ); x = ( - First - , - Second + First,Second ); @@ -227,19 +262,13 @@ x = ( x = ( - {hour} - : - {minute} - : - {second} + {hour}:{minute}:{second} ); x = ( - {hour}: - {minute}: - {second} + {hour}:{minute}:{second} ); diff --git a/crates/rome_js_formatter/tests/specs/prettier/jsx/text-wrap/test.js.snap b/crates/rome_js_formatter/tests/specs/prettier/jsx/text-wrap/test.js.snap index a5a890515de..8f4dbc7296f 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/jsx/text-wrap/test.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/jsx/text-wrap/test.js.snap @@ -494,65 +494,6 @@ let myDiv = ReactTestUtils.renderIntoDocument(
First
Second
-@@ -436,13 +441,16 @@ - - x = ( -
--
First
-
Second
-+
First
- -+
Second
-
- ); - - x = ( -
--
First
-
Second
-+
First
-+ - -+
Second
-
- ); - -@@ -496,19 +504,26 @@ - - x = ( -
-- {hour}:{minute}:{second} -+ {hour} -+ : -+ {minute} -+ : -+ {second} -
- ); - - x = ( -
-- {hour}:{minute}:{second} -+ {hour}: -+ {minute}: -+ {second} -
- ); - - x = ( -
-- text here.
-+ text here. -+
-
- ); - -@@ -528,7 +543,8 @@ - - {name}’s{" "} - -- Hello world.
-+ Hello world. -+
- You {type}ed this shipment to -
- ); ``` # Output @@ -1001,16 +942,13 @@ x = ( x = (
-
First
- -
Second
+
First
-
Second
); x = (
-
First
- - -
Second
+
First
-
Second
); @@ -1064,26 +1002,19 @@ x = ( x = (
- {hour} - : - {minute} - : - {second} + {hour}:{minute}:{second}
); x = (
- {hour}: - {minute}: - {second} + {hour}:{minute}:{second}
); x = (
- text here. -
+ text here.
); @@ -1103,8 +1034,7 @@ x = ( {name}’s{" "} - Hello world. -
+ Hello world.
You {type}ed this shipment to
);