mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
main
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Lee Jaeha / Adamandleaysgur
|
255c5cca26 |
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> |
||
|
Lee Jaeha / Adamandleaysgur
|
0b441649eb |
fix(formatter): decide JSX opening element layout from collapsed Tailwind class string (#28744)
## 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 = (
<div
className="p-4
flex"
>
Hello
</div>
);
```
1st run:
```tsx
const A = (
<div
className="flex p-4"
>
Hello
</div>
);
```
2nd run:
```tsx
const A = <div className="flex p-4">Hello</div>;
```
### 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>
|