From 0b441649eb57abaa1f22661a3ceb7334a2e29fd7 Mon Sep 17 00:00:00 2001 From: Lee Jaeha / Adam Date: Wed, 4 Nov 2026 13:46:43 +0900 Subject: [PATCH] fix(formatter): decide JSX opening element layout from collapsed Tailwind class string (#28744) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary With `sortTailwindcss` enabled (and `preserveWhitespace` off), formatting a JSX element whose Tailwind class string spans multiple lines is not idempotent. It takes two runs to reach a stable output. Input (`{"sortTailwindcss": {}}`): ```tsx const A = (
Hello
); ``` 1st run: ```tsx const A = (
Hello
); ``` 2nd run: ```tsx const A =
Hello
; ``` ### Cause `FormatOpeningElement` decides its layout from the attribute's **source** string value: - `SingleStringAttribute` is used only if the value has no line break. - Otherwise the attributes group is forced to expand if any string value has a line break. But for a Tailwind attribute without `preserveWhitespace`, `write_tailwind_string_literal` collapses all whitespace (including line breaks) into single spaces. So the printed value has no line break while the layout was decided as if it had one. The next run sees a single-line value and joins the element. prettier-plugin-tailwindcss rewrites the attribute value in the AST before printing, so Prettier makes the layout decision on the collapsed value and gets the 2nd-run output in one pass. ### Fix Decide the layout from whether the **printed** value has a line break. A Tailwind attribute (per `is_tailwind_jsx_attribute`) with `preserveWhitespace` off is treated as single-line. Nothing changes when `sortTailwindcss` is off, with `preserveWhitespace: true`, or for non-Tailwind attributes. ## Test - Added 3 cases to `apps/oxfmt/test/api/sort_tailwindcss.test.ts`: - multiline class string in a single-attribute and a multi-attribute element: formats in one run, and a 2nd run doesn't change the output. Fails without this fix. - `preserveWhitespace: true`: still keeps line breaks and expands the element (unchanged). - multiline non-Tailwind attribute (`title`) next to a `className`: still expands the element (unchanged). - `apps/oxfmt`: `vitest run` → 553 passed - `cargo test -p oxc_formatter`, `CARGO_BUILD_WARNINGS=deny cargo lint -p oxc_formatter`, `cargo fmt --check`, `node --run fmt` pass - I didn't run `conformance:prettier` (fixtures not downloaded locally). It doesn't enable `sortTailwindcss`, so this code path behaves the same as before there. - Found while migrating a real Next.js app to oxfmt: 42 files needed a second `oxfmt --write` run. --------- Co-authored-by: leaysgur <6259812+leaysgur@users.noreply.github.com> --- apps/oxfmt/test/api/sort_tailwindcss.test.ts | 94 +++++++++++++++++++ crates/oxc_formatter/src/print/jsx/mod.rs | 12 +-- .../src/print/jsx/opening_element.rs | 67 ++++++------- crates/oxc_formatter/src/utils/tailwindcss.rs | 16 +++- 4 files changed, 140 insertions(+), 49 deletions(-) diff --git a/apps/oxfmt/test/api/sort_tailwindcss.test.ts b/apps/oxfmt/test/api/sort_tailwindcss.test.ts index 8c3cbb550f..af7a9eab9e 100644 --- a/apps/oxfmt/test/api/sort_tailwindcss.test.ts +++ b/apps/oxfmt/test/api/sort_tailwindcss.test.ts @@ -566,6 +566,100 @@ shadow-lg\` : "font-normal"}\`} />;`; expect(result.errors).toStrictEqual([]); }); + it("should not break JSX element for multiline class string that gets collapsed", async () => { + const input = `const A = ( +
+ Hello +
+); +const B = ( +
+ Hello +
+); +`; + + const result = await format("test.tsx", input, { + experimentalTailwindcss: {}, + }); + + // The value is collapsed into one line, so the element should not be expanded + // as if the value were still multiline. + expect(result.code).toMatchInlineSnapshot(` + "const A =
Hello
; + const B = ( +
+ Hello +
+ ); + " + `); + expect(result.errors).toStrictEqual([]); + + // Formatting the output again should not change it. + const second = await format("test.tsx", result.code, { + experimentalTailwindcss: {}, + }); + expect(second.code).toBe(result.code); + }); + + it("should still break JSX element for multiline class string when preserveWhitespace is true", async () => { + const input = `const A = ( +
+ Hello +
+); +`; + + const result = await format("test.tsx", input, { + experimentalTailwindcss: { preserveWhitespace: true }, + }); + + expect(result.code).toBe(input); + expect(result.errors).toStrictEqual([]); + }); + + it("should still break JSX element for multiline non-Tailwind string attribute", async () => { + const input = `const A = ( +
+ Hello +
+); +`; + + const result = await format("test.tsx", input, { + experimentalTailwindcss: {}, + }); + + expect(result.code).toMatchInlineSnapshot(` + "const A = ( +
+ Hello +
+ ); + " + `); + expect(result.errors).toStrictEqual([]); + }); + // Tests for can_collapse_whitespace in template literal expressions // These test the whitespace preservation logic based on adjacent quasi whitespace diff --git a/crates/oxc_formatter/src/print/jsx/mod.rs b/crates/oxc_formatter/src/print/jsx/mod.rs index 2bcbe80fb8..3a1c00148d 100644 --- a/crates/oxc_formatter/src/print/jsx/mod.rs +++ b/crates/oxc_formatter/src/print/jsx/mod.rs @@ -15,13 +15,12 @@ use crate::{ ast_nodes::{AstNode, AstNodes}, format_args, formatter::{ - TailwindContextEntry, prelude::*, trivia::{DanglingIndentMode, FormatDanglingComments}, }, utils::{ expression::as_call_expression_without_chain_wrappers, - tailwindcss::is_tailwind_jsx_attribute, + tailwindcss::tailwind_context_for_jsx_attribute, }, write, }; @@ -300,14 +299,7 @@ impl<'a> FormatWrite<'a> for AstNode<'_, 'a, JSXAttribute<'a>> { write!(f, self.name()); if let Some(value) = &self.value() { - // Check if this is a Tailwind attribute and push context - // Extract context entry before mutating f - let tailwind_ctx_to_push = f - .options() - .sort_tailwindcss - .as_ref() - .filter(|opts| is_tailwind_jsx_attribute(&self.name, opts)) - .map(|opts| TailwindContextEntry::new(opts.preserve_whitespace)); + let tailwind_ctx_to_push = tailwind_context_for_jsx_attribute(&self.name, f); if let Some(ctx) = tailwind_ctx_to_push { f.context_mut().push_tailwind_context(ctx); diff --git a/crates/oxc_formatter/src/print/jsx/opening_element.rs b/crates/oxc_formatter/src/print/jsx/opening_element.rs index 01fa114fb4..56033bd922 100644 --- a/crates/oxc_formatter/src/print/jsx/opening_element.rs +++ b/crates/oxc_formatter/src/print/jsx/opening_element.rs @@ -1,9 +1,12 @@ use std::ops::Deref; -use oxc_ast::ast::{JSXAttributeItem, JSXAttributeValue, JSXOpeningElement, StringLiteral}; +use oxc_ast::ast::{JSXAttributeItem, JSXAttributeValue, JSXOpeningElement}; use oxc_span::GetSpan; -use crate::{ast_nodes::AstNode, formatter::prelude::*, write}; +use crate::{ + ast_nodes::AstNode, formatter::prelude::*, + utils::tailwindcss::tailwind_context_for_jsx_attribute, write, +}; pub struct FormatOpeningElement<'a, 'b> { element: &'b AstNode<'b, 'a, JSXOpeningElement<'a>>, @@ -45,7 +48,7 @@ impl<'a, 'b> FormatOpeningElement<'a, 'b> { } else if attributes.len() == 1 && !name_has_comment && !last_attribute_has_comment - && is_single_line_string_literal_attribute(&attributes[0]) + && string_literal_attribute_has_line_break(&attributes[0], f) == Some(false) { OpeningElementLayout::SingleStringAttribute } else { @@ -54,17 +57,28 @@ impl<'a, 'b> FormatOpeningElement<'a, 'b> { } } -/// Returns `true` if this is an attribute with a [`StringLiteral`] initializer that contains at least one new line character. -fn is_multiline_string_literal_attribute(attribute: &JSXAttributeItem<'_>) -> bool { +/// Whether this attribute's string literal value is printed with a line break, +/// or `None` if the value isn't a string literal. +/// - Tailwind sorting without `preserveWhitespace` collapses line breaks, +/// so checking the source value alone would make formatting non-idempotent +fn string_literal_attribute_has_line_break( + attribute: &JSXAttributeItem<'_>, + f: &JsFormatter<'_, '_>, +) -> Option { let JSXAttributeItem::Attribute(attr) = attribute else { - return false; + return None; }; - attr.value.as_ref().is_some_and(|value| { - matches!( - value, - JSXAttributeValue::StringLiteral(s) if s.value.contains(|c: char| matches!(c, '\n' | '\r')) - ) - }) + let Some(JSXAttributeValue::StringLiteral(string)) = &attr.value else { + return None; + }; + + if !string.value.contains(|c: char| matches!(c, '\n' | '\r')) { + return Some(false); + } + + let collapses_whitespace = tailwind_context_for_jsx_attribute(&attr.name, f) + .is_some_and(|ctx| !ctx.preserve_whitespace); + Some(!collapses_whitespace) } impl<'a> Format<'a, JsFormatContext<'a>> for FormatOpeningElement<'a, '_> { @@ -115,9 +129,9 @@ impl<'a> Format<'a, JsFormatContext<'a>> for FormatOpeningElement<'a, '_> { } }); - let has_multiline_string_attribute = attributes - .iter() - .any(|attribute| is_multiline_string_literal_attribute(&attribute)); + let has_multiline_string_attribute = attributes.iter().any(|attribute| { + string_literal_attribute_has_line_break(&attribute, f) == Some(true) + }); write!(f, [group(&format_inner).should_expand(has_multiline_string_attribute)]); } } @@ -153,26 +167,3 @@ pub enum OpeningElementLayout { /// ``` IndentAttributes { name_has_comment: bool, last_attribute_has_comment: bool }, } - -/// Returns `true` if this is an attribute with a string literal initializer that does not contain any new line characters. -fn is_single_line_string_literal_attribute(attribute: &JSXAttributeItem) -> bool { - as_string_literal_attribute_value(attribute) - .is_some_and(|string| !string.value.contains(|c: char| matches!(c, '\n' | '\r'))) -} - -/// Returns `Some` if the initializer value of this attribute is a string literal. -/// Returns [None] otherwise. -fn as_string_literal_attribute_value<'a>( - attribute: &'a JSXAttributeItem<'a>, -) -> Option<&'a StringLiteral<'a>> { - match attribute { - JSXAttributeItem::Attribute(attr) => { - if let Some(JSXAttributeValue::StringLiteral(string)) = &attr.value { - Some(string.as_ref()) - } else { - None - } - } - JSXAttributeItem::SpreadAttribute(_) => None, - } -} diff --git a/crates/oxc_formatter/src/utils/tailwindcss.rs b/crates/oxc_formatter/src/utils/tailwindcss.rs index 75c371c661..7756a4ef1d 100644 --- a/crates/oxc_formatter/src/utils/tailwindcss.rs +++ b/crates/oxc_formatter/src/utils/tailwindcss.rs @@ -50,7 +50,7 @@ pub fn tailwind_context_for_string_literal<'a>( /// Returns `true` for: /// - `class` and `className` (default attributes) /// - Custom attributes specified in `attributes` option -pub fn is_tailwind_jsx_attribute( +fn is_tailwind_jsx_attribute( attr_name: &JSXAttributeName<'_>, options: &SortTailwindcssOptions, ) -> bool { @@ -68,6 +68,20 @@ pub fn is_tailwind_jsx_attribute( options.attributes.iter().any(|a| a == name) } +/// Returns the Tailwind context to push while printing the value of the JSX attribute `attr_name`. +/// +/// Also read by the opening element layout, which must agree with the print on whether the value is collapsed. +pub fn tailwind_context_for_jsx_attribute( + attr_name: &JSXAttributeName<'_>, + f: &JsFormatter<'_, '_>, +) -> Option { + f.options() + .sort_tailwindcss + .as_ref() + .filter(|opts| is_tailwind_jsx_attribute(attr_name, opts)) + .map(|opts| TailwindContextEntry::new(opts.preserve_whitespace)) +} + /// Checks if a callee expression is a Tailwind function. /// /// Traverses through `CallExpression` and `MemberExpression` nodes to find the