mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(formatter): measure Tailwind class argument after collapsing whitespace (#28745)
## Summary Follow-up to #28744, same kind of issue in a different place. With `sortTailwindcss` enabled, an assignment whose right side is a Tailwind function call can take two runs to reach a stable output, because the "short argument" check measures the argument's **source** text while the printed text has its whitespace collapsed. Input (`{"sortTailwindcss": {"functions": ["cn"]}}`, default `printWidth: 100`): ```ts const vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = cn("p-4 flex"); ``` 1st run: ```ts const vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = cn( "flex p-4", ); ``` 2nd run (also what Prettier + prettier-plugin-tailwindcss prints in one run): ```ts const vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = cn("flex p-4"); ``` The same happens for `a.b = cn(...)`, `{ key: cn(...) }` and `cn(...).trim()`. ### Cause `is_poorly_breakable_member_or_call_chain` uses `is_short_argument` (Prettier's `isLoneShortArgument`) to check if the only argument is at most `printWidth / 4` wide. For a string or template literal it measures the source text. When the argument is sorted as Tailwind classes, whitespace is collapsed when printed, so the source can be "long" while the printed text is "short". Prettier sees the already rewritten value, so it decides on the printed text. ### Fix When the call is a Tailwind function call and `preserveWhitespace` is off, measure the string / template literal argument after collapsing whitespace (`collapsed_class_list_width`). ### Known limitation: duplicate classes Duplicate removal also shortens the printed text, e.g. `cn("flex flex flex flex flex p-4")` still takes two runs. I didn't handle it, because the sorter removes duplicates only for classes known to Tailwind CSS (`sortClassList` in prettier-plugin-tailwindcss: `if (order !== null) seenClasses.add(cls)`), and the formatter can't know which classes are known while building the IR. The options I could think of: 1. Leave it (this PR). Duplicates are rare in real code, and the result is still valid. It just settles on the 2nd run. 2. Also dedupe all classes when measuring. This fixes the common case but can measure too short when unknown classes are duplicated. 3. Make the sorted text available before the layout decision, which is a larger change to how sorting is batched. I went with 1 to keep this small. If you prefer another approach, please let me know and I'll update it. ## Test - Added cases to `apps/oxfmt/test/api/sort_tailwindcss.test.ts`: - string and template literal arguments with extra whitespace format in one run, and a 2nd run doesn't change the output. These fail without this fix. - `preserveWhitespace: true`: the source text is still measured (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 also formatted 7 code shapes × 5 class strings × identifier lengths 1–100 twice and compared the runs. All whitespace cases are now stable; only the duplicate-class cases above remain. --------- Co-authored-by: leaysgur <6259812+leaysgur@users.noreply.github.com>
This commit is contained in:
1 parent
ea76490ca5
commit
255c5cca26
3 files changed
+82
-17
No files matched your search
@@ -566,6 +566,23 @@ shadow-lg\` : "font-normal"}\`} />;`;
|
||||
expect(result.errors).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("should measure the argument with whitespace when preserveWhitespace is true", async () => {
|
||||
const input = `const vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = cn("p-4 flex");\n`;
|
||||
|
||||
const result = await format("test.ts", input, {
|
||||
experimentalTailwindcss: { functions: ["cn"], preserveWhitespace: true },
|
||||
});
|
||||
|
||||
// Whitespace is kept, so the argument is long and the call is broken.
|
||||
expect(result.code).toMatchInlineSnapshot(`
|
||||
"const vaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = cn(
|
||||
"flex p-4",
|
||||
);
|
||||
"
|
||||
`);
|
||||
expect(result.errors).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("should not break JSX element for multiline class string that gets collapsed", async () => {
|
||||
const input = `const A = (
|
||||
<div
|
||||
@@ -660,7 +677,7 @@ const B = (
|
||||
expect(result.errors).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("should decide layouts from the collapsed template literal and object key", async () => {
|
||||
it("should decide layouts from collapsed Tailwind classes", async () => {
|
||||
const input = `const a = clsx(\`
|
||||
p-4 flex items-center justify-between \${active ? "bg-white" : "bg-black"} text-black hover:bg-gray-100
|
||||
\`);
|
||||
@@ -671,13 +688,16 @@ const b = () => tw\`
|
||||
p-4 flex items-center justify-between bg-white text-black hover:bg-gray-100
|
||||
\`;
|
||||
const c = clsx({ "a b": someVeryLongConditionIdentifierNameThatIsReallyQuiteLongToForceBreakingHereAndMore });
|
||||
const ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd = clsx("p-4 flex");
|
||||
const eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee = clsx(\`p-4 flex\`);
|
||||
const f = clsx({ kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk: foo(\`p-4 flex\`) });
|
||||
`;
|
||||
const options = { tabWidth: 4, experimentalTailwindcss: { functions: ["clsx", "tw"] } };
|
||||
|
||||
const result = await format("test.ts", input, options);
|
||||
|
||||
// The line breaks are collapsed, so the template is not hugged as multiline,
|
||||
// and the collapsed key is short.
|
||||
// and the collapsed key and call arguments are short.
|
||||
expect(result.code).toMatchInlineSnapshot(`
|
||||
"const a = clsx(
|
||||
\`flex items-center justify-between p-4 \${active ? "bg-white" : "bg-black"} text-black hover:bg-gray-100\`,
|
||||
@@ -689,6 +709,14 @@ const c = clsx({ "a b": someVeryLongConditionIdentifierNameThatIsReallyQuiteLo
|
||||
const c = clsx({
|
||||
"a b": someVeryLongConditionIdentifierNameThatIsReallyQuiteLongToForceBreakingHereAndMore,
|
||||
});
|
||||
const ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd =
|
||||
clsx("flex p-4");
|
||||
const eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee =
|
||||
clsx(\`flex p-4\`);
|
||||
const f = clsx({
|
||||
kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk:
|
||||
foo(\`flex p-4\`),
|
||||
});
|
||||
"
|
||||
`);
|
||||
expect(result.errors).toStrictEqual([]);
|
||||
|
||||
@@ -21,6 +21,10 @@ use crate::{
|
||||
format_node_without_trailing_comments::FormatNodeWithoutTrailingComments,
|
||||
member_chain::is_member_call_chain,
|
||||
object::{format_property_key, write_member_name},
|
||||
tailwindcss::{
|
||||
collapsed_class_list_width, tailwind_collapses_template_line_breaks,
|
||||
tailwind_context_for_callee,
|
||||
},
|
||||
typecast::is_cast_target,
|
||||
},
|
||||
write,
|
||||
@@ -1211,7 +1215,7 @@ fn is_poorly_breakable_member_or_call_chain<'a>(
|
||||
// An embedded template is formatted, so its source length says nothing
|
||||
Some(first_argument) => first_argument.as_expression().is_none_or(|e| {
|
||||
embed_hug(e, Some(call_expression), f).is_some()
|
||||
|| !is_short_argument(e, threshold, f)
|
||||
|| !is_short_argument(e, threshold, &call_expression.callee, f)
|
||||
}),
|
||||
None => false,
|
||||
},
|
||||
@@ -1238,11 +1242,19 @@ fn is_poorly_breakable_member_or_call_chain<'a>(
|
||||
/// We need it to decide if [`CallExpression`] with the argument is breakable or not
|
||||
/// If the argument is short the function call isn't breakable
|
||||
/// [Prettier applies]: <https://github.com/prettier/prettier/blob/0273e33fc691e28e4ab3f3c8ee86918b65cf823d/src/language-js/utils/index.js#L433-L484>
|
||||
fn is_short_argument(argument: &Expression, threshold: u16, f: &JsFormatter) -> bool {
|
||||
///
|
||||
/// A string or template whose whitespace is collapsed by Tailwind sorting under `callee`
|
||||
/// is measured as printed, as Prettier measures the plugin's output.
|
||||
fn is_short_argument(
|
||||
argument: &Expression,
|
||||
threshold: u16,
|
||||
callee: &Expression,
|
||||
f: &JsFormatter,
|
||||
) -> bool {
|
||||
match argument {
|
||||
Expression::Identifier(identifier) => identifier.name.width() <= threshold as usize,
|
||||
Expression::UnaryExpression(unary_expression) => {
|
||||
is_short_argument(&unary_expression.argument, threshold, f)
|
||||
is_short_argument(&unary_expression.argument, threshold, callee, f)
|
||||
}
|
||||
Expression::RegExpLiteral(regex) => regex.regex.pattern.text.width() <= threshold as usize,
|
||||
Expression::StringLiteral(literal) => {
|
||||
@@ -1251,8 +1263,17 @@ fn is_short_argument(argument: &Expression, threshold: u16, f: &JsFormatter) ->
|
||||
false,
|
||||
StringLiteralParentKind::Expression,
|
||||
);
|
||||
let text = formatter.clean_text(f);
|
||||
|
||||
formatter.clean_text(f).width() <= threshold as usize
|
||||
let width = if tailwind_context_for_callee(callee, f)
|
||||
.is_some_and(|ctx| !ctx.preserve_whitespace)
|
||||
{
|
||||
// Opening and closing quotes, and the classes
|
||||
2 + collapsed_class_list_width(&text[1..text.len() - 1])
|
||||
} else {
|
||||
text.width()
|
||||
};
|
||||
width <= threshold as usize
|
||||
}
|
||||
Expression::TemplateLiteral(literal) => {
|
||||
// Besides checking length exceed we also need to check that the template doesn't have any expressions.
|
||||
@@ -1260,7 +1281,11 @@ fn is_short_argument(argument: &Expression, threshold: u16, f: &JsFormatter) ->
|
||||
// Prettier: https://github.com/prettier/prettier/blob/a043ac0d733c4d53f980aa73807a63fc914f23bd/src/language-js/print/assignment.js#L402-L405
|
||||
literal.quasis.len() == 1 && {
|
||||
let raw = literal.quasis[0].value.raw;
|
||||
raw.width() <= threshold as usize && !raw.contains('\n')
|
||||
if tailwind_collapses_template_line_breaks(Some(callee), f) {
|
||||
collapsed_class_list_width(&raw) <= threshold as usize
|
||||
} else {
|
||||
raw.width() <= threshold as usize && !raw.contains('\n')
|
||||
}
|
||||
}
|
||||
}
|
||||
Expression::CallExpression(call) => {
|
||||
|
||||
@@ -84,16 +84,17 @@ pub fn tailwind_context_for_jsx_attribute(
|
||||
.map(|opts| TailwindContextEntry::new(opts.preserve_whitespace))
|
||||
}
|
||||
|
||||
/// Whether a template literal printed at this point (under `tag` when tagged) has its line breaks collapsed.
|
||||
/// - Reads what `TemplateElement` reads: the current Tailwind context, or the one a Tailwind `tag` pushes
|
||||
/// Whether a template literal printed at this point has its whitespace, including line breaks, collapsed.
|
||||
/// - `callee` is the tag of a tagged template, or the callee of the call it's an argument of
|
||||
/// - Reads what `TemplateElement` reads: the current Tailwind context, or the one a Tailwind `callee` pushes
|
||||
pub fn tailwind_collapses_template_line_breaks(
|
||||
tag: Option<&Expression<'_>>,
|
||||
callee: Option<&Expression<'_>>,
|
||||
f: &JsFormatter<'_, '_>,
|
||||
) -> bool {
|
||||
f.context()
|
||||
.tailwind_context()
|
||||
.copied()
|
||||
.or_else(|| tag.and_then(|tag| tailwind_context_for_callee(tag, f)))
|
||||
.or_else(|| callee.and_then(|callee| tailwind_context_for_callee(callee, f)))
|
||||
.is_some_and(|ctx| !ctx.preserve_whitespace)
|
||||
}
|
||||
|
||||
@@ -151,6 +152,19 @@ fn is_tailwind_function_call(callee: &Expression<'_>, options: &SortTailwindcssO
|
||||
}
|
||||
}
|
||||
|
||||
/// Printed width of a class list with collapsed whitespace:
|
||||
/// trimmed, then split on ASCII whitespace and joined with single spaces, as the sorter does.
|
||||
/// - Whitespace-only content is printed as a single space
|
||||
/// - An upper bound: the sorter also drops duplicates of known classes
|
||||
pub fn collapsed_class_list_width(content: &str) -> usize {
|
||||
let trimmed = content.trim();
|
||||
if trimmed.is_empty() {
|
||||
return usize::from(!content.is_empty());
|
||||
}
|
||||
// Each class plus its separator, minus the last separator
|
||||
trimmed.split_ascii_whitespace().map(|class| class.width() + 1).sum::<usize>() - 1
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Whitespace Collapse Logic
|
||||
// ============================================================================
|
||||
@@ -323,7 +337,7 @@ pub fn write_tailwind_string_literal<'a>(
|
||||
}
|
||||
write!(f, quote);
|
||||
// Quotes, and the single space the content is normalized to
|
||||
return 2 + usize::from(!content.is_empty());
|
||||
return 2 + collapsed_class_list_width(content);
|
||||
}
|
||||
|
||||
let collapse = can_collapse_whitespace(string_literal.span, string_literal.ancestors(), f);
|
||||
@@ -347,12 +361,10 @@ pub fn write_tailwind_string_literal<'a>(
|
||||
}
|
||||
write!(f, quote);
|
||||
|
||||
// The sorter joins the classes with single spaces, splitting on ASCII whitespace:
|
||||
// each class plus its separator, minus the last separator (`trimmed` has at least one class)
|
||||
let joined_width =
|
||||
trimmed.split_ascii_whitespace().map(|class| class.width() + 1).sum::<usize>() - 1;
|
||||
// Quotes, the kept boundary spaces, and the classes
|
||||
2 + usize::from(keeps_leading_space) + joined_width + usize::from(keeps_trailing_space)
|
||||
2 + usize::from(keeps_leading_space)
|
||||
+ collapsed_class_list_width(trimmed)
|
||||
+ usize::from(keeps_trailing_space)
|
||||
}
|
||||
|
||||
/// Writes a template element (quasi) with Tailwind class sorting.
|
||||
|
||||
Reference in new issue
Block a user