fix(rome_js_formatter): jsx punctuation
Fix
MODIFY
crates/rome_js_formatter/src/jsx/lists/child_list.rs
crates/rome_js_formatter/src/jsx/lists/child_list.rs
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<JsxChild> = 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,10 +165,51 @@ 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]
+ // ```
+ // <div>
+ // <div>First</div>,
+ // <div>Second</div>
+ // </div>
+ // ```
+ 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]
+ // ```
+ // <div>
+ // <div>First</div>
+ // ,<div>Second</div>
+ // </div>
+ // ```
+ 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
+ }
+ };
+ 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
JsxChild::EmptyLine => {
@@ -406,7 +447,8 @@ enum WordSeparator {
/// <div>a{expression}</div> // 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 = <div>ab<br/></div>;
@@ -420,10 +462,7 @@ enum WordSeparator {
/// </div>
/// );
/// ```
- 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<JsFormatContext> for WordSeparator {
fn fmt(&self, f: &mut Formatter<JsFormatContext>) -> 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
// <div>ab<br/></div>
// ```
@@ -456,12 +496,8 @@ impl Format<JsFormatContext> for WordSeparator {
// <br />
// </div>
// ```
- 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)
}
}
}

MODIFY
crates/rome_js_formatter/src/utils/jsx.rs
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
+ let text = value_token
.token_text()
- .slice(TextRange::at(relative_start, word.text_len())),
- source_position: value_token.text_range().start() + relative_start,
- }));
+ .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<I: Iterator> {
+ iter: I,
+
+ peeked: Option<Option<I::Item>>,
+ peeked_next: Option<Option<I::Item>>,
+}
+
+impl<I: Iterator> JsxChildrenIterator<I> {
+ 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<I: Iterator> Iterator for JsxChildrenIterator<I> {
+ type Item = I::Item;
+
+ fn next(&mut self) -> Option<Self::Item> {
+ 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}</>"),

ADD
crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx
+
+
+x = (
+ <div>
+ <div>First</div>,<div>Second</div>
+ </div>
+);
+
+x = (
+ <div>
+ <div>First</div>,
+ <div>Second</div>
+ </div>
+);
+
+x = (
+ <div>
+ <div>First</div>
+ ,<div>Second</div>
+ </div>
+);
+
+function Component() {
+ return (
+ <>
+ <span>text</span>.<br />
+ </>
+ );
+}
+
+let myDiv1 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />,<di key="theBird" className="bird" />
+ </div1>
+);
+
+
+let myDiv2 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />
+ ,<di key="theBird" className="bird" />
+ </div1>
+);
+
+let myDiv3 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />,
+ <di key="theBird" className="bird" />
+ </div1>
+);

ADD
crates/rome_js_formatter/tests/specs/jsx/new-lines.jsx.snap
+---
+source: crates/rome_js_formatter/tests/spec_test.rs
+expression: new-lines.jsx
+---
+
+# Input
+
+```js
+
+
+x = (
+ <div>
+ <div>First</div>,<div>Second</div>
+ </div>
+);
+
+x = (
+ <div>
+ <div>First</div>,
+ <div>Second</div>
+ </div>
+);
+
+x = (
+ <div>
+ <div>First</div>
+ ,<div>Second</div>
+ </div>
+);
+
+function Component() {
+ return (
+ <>
+ <span>text</span>.<br />
+ </>
+ );
+}
+
+let myDiv1 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />,<di key="theBird" className="bird" />
+ </div1>
+);
+
+
+let myDiv2 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />
+ ,<di key="theBird" className="bird" />
+ </div1>
+);
+
+let myDiv3 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />,
+ <di key="theBird" className="bird" />
+ </div1>
+);
+
+```
+
+
+=============================
+
+# Outputs
+
+## Output 1
+
+-----
+Indent style: Tab
+Line width: 80
+Quote style: Double Quotes
+Quote properties: As needed
+Trailing comma: All
+Semicolons: Always
+-----
+
+```js
+x = (
+ <div>
+ <div>First</div>,<div>Second</div>
+ </div>
+);
+
+x = (
+ <div>
+ <div>First</div>,<div>Second</div>
+ </div>
+);
+
+x = (
+ <div>
+ <div>First</div>,<div>Second</div>
+ </div>
+);
+
+function Component() {
+ return (
+ <>
+ <span>text</span>.<br />
+ </>
+ );
+}
+
+let myDiv1 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />,<di key="theBird" className="bird" />
+ </div1>,
+);
+
+let myDiv2 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />
+ ,<di key="theBird" className="bird" />
+ </div1>,
+);
+
+let myDiv3 = ReactTestUtils.renderIntoDocument(
+ <div1>
+ <div key="theDog" className="dog" />,
+ <di key="theBird" className="bird" />
+ </div1>,
+);
+```
+
+

MODIFY
crates/rome_js_formatter/tests/specs/prettier/jsx/fbt/test.js.snap
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 = (
+ <fbt>
+- <fbt:param>First</fbt:param>,
+- <fbt:param>Second</fbt:param>
++ <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
+ </fbt>
+ );
+
+ x = (
+ <fbt>
+- <fbt:param>First</fbt:param>
+- ,
+- <fbt:param>Second</fbt:param>
++ <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
+ </fbt>
+ );
+
+@@ -62,33 +59,35 @@
+
+ x = (
+ <fbt>
+- {hour}
+- :
+- {minute}
+- :
+- {second}
++ {hour}:{minute}:{second}
+ </fbt>
+ );
+
+ x = (
+ <fbt>
+- {hour}:
+- {minute}:
+- {second}
++ {hour}:{minute}:{second}
+ </fbt>
+ );
first = (
<fbt>
@@ -165,16 +203,13 @@ third = (
```js
x = (
<fbt>
- <fbt:param>First</fbt:param>,
- <fbt:param>Second</fbt:param>
+ <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
</fbt>
);
x = (
<fbt>
- <fbt:param>First</fbt:param>
- ,
- <fbt:param>Second</fbt:param>
+ <fbt:param>First</fbt:param>,<fbt:param>Second</fbt:param>
</fbt>
);
@@ -227,19 +262,13 @@ x = (
x = (
<fbt>
- {hour}
- :
- {minute}
- :
- {second}
+ {hour}:{minute}:{second}
</fbt>
);
x = (
<fbt>
- {hour}:
- {minute}:
- {second}
+ {hour}:{minute}:{second}
</fbt>
);

fix(rome_js_formatter): jsx punctuation (#3842)

Fix https://github.com/rome/tools/issues/3531#issuecomment-1325080671