feat(useSortedClasses): scaffold v4 parser-based sort module#10291
feat(useSortedClasses): scaffold v4 parser-based sort module#10291jiwon79 wants to merge 23 commits into
Conversation
Generates `tailwind_preset_v4.rs` by probing a v4 design system at codegen time (884 static + 311 functional utilities, 354 property-order entries, 18 theme namespace key sets). The Rust file is currently unwired; value predicates and sort logic land next.
Emits each `FunctionalEntry` on multiple lines (one branch per line) and orders branches by resolve precedence (NamedKeyword → Named → NamedTyped → Typed → Arbitrary) so the generated table reads top-to-bottom in the order the runtime lookup will try.
…rated preset Drops the `use ValueType::*; use ThemeNamespace::*;` re-exports; both enums share variant names (e.g. `Color`), so glob-importing them caused ambiguous-name + type-mismatch errors. The generator now emits fully qualified `ValueType::<X>` and `ThemeNamespace::<X>` paths.
Adds `predicates.rs` with 17 CSS value-type checks (`is_color`, `is_length`, `is_percentage`, ...) ported 1:1 from Tailwind v4.2.2. Uses `LazyLock<Regex>` for the seven anchored numeric patterns, `phf::Set` for the 169 named CSS colors, a custom `Segments` iterator backed by `SmallVec` for top-level CSS splitting (no heap alloc on hot paths), and a 32-byte stack buffer for case-insensitive named-color lookup. Each predicate carries a doc-link to the matching Tailwind function. Wires up `predicates` and `tailwind_preset_v4` in the rule's mod tree and adds `phf` to the crate's workspace dependencies. Inline tests cover each predicate (16 cases, all passing).
Adds sort_v4::sort_class_list(&TwRoot) -> String as a parser-based passthrough stub, paired with a jsonc fixture and insta::glob! runner. The real v4 sort algorithm will replace the body in a follow-up.
…enericName - Branch::Typed → Branch::ArbitraryTyped to make the bracketed dispatch / fallback pair (ArbitraryTyped / Arbitrary) read as a parallel family. - Drop ValueType::FamilyName and ::GenericName: their probes always dedupe against the Arbitrary fallback for `font-` (both route to font-family), so the variants and predicates were unreachable. - Replace #![allow(dead_code)] with #[expect(dead_code, reason = ...)] to satisfy clippy::allow_attributes; tighten &'static str redundancy.
- Replace free `classify` with `SortKey::from_candidate(&AnyTwFullCandidate)` so the Bogus and Full arms collapse into one method and the caller no longer rebuilds a SortKey from a tuple. - Replace `resolve_value -> Option<(u16, u8)>` with `resolve_branch -> Option<SortKey>`, threading `registration_idx` through so the helper returns a complete sort key. - Pre-size the `Vec` and the output `String` so neither reallocates during a single sort pass. - Use `.contains` and `usize::from(pool_idx)` to drop the `as` cast and the redundant `iter().any(...)`. - Extend the fixture with font / w / aspect bare-value cases that exercise Named, NamedKeyword, and NamedTyped(Number/Ratio).
…sts/ Match Biome's convention of keeping integration tests under `tests/`. The runner now lives in `tests/sort_v4_test.rs` and reads fixtures from `tests/sort_v4/`, with snapshots emitted flat next to the fixture (`cases.jsonc` → `cases.snap`). The inline `#[cfg(test)]` block under `src/` is removed.
🦋 Changeset detectedLatest commit: ccbccc5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
A-Linter
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds Tailwind v4 support: a TypeScript pipeline that extracts v4 property order, theme keys and utilities and renders generated Rust preset source; Rust-side adds nesting-aware CSS value predicates, a v4 sort implementation computing stable SortKeys (unknowns preserved at front), Cargo wiring for new parser/syntax crates, and snapshot-driven tests with fixtures. Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/biome_js_analyze/tests/sort_v4_test.rs (1)
13-15: 💤 Low valueConsider surfacing parse errors in test output
parse_tailwindparse errors are silently ignored — the sorted output could be partially or fully derived from an error-recovered tree. Even just includingparsed.diagnostics()in the rendered snapshot would make future debugging much easier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_js_analyze/tests/sort_v4_test.rs` around lines 13 - 15, The test currently ignores parse errors from parse_tailwind and may assert against an error-recovered tree; change the test to include parse diagnostics in the snapshot output by appending parsed.diagnostics() (or formatted diagnostics) to the rendered string alongside the existing parsed.tree()/sorted result, so when calling parse_tailwind and sort_class_list you render something like the input, parsed.diagnostics(), and sorted to make parse failures visible during test failures.packages/tailwindcss-config-analyzer/src/v4/extract-property-order.ts (1)
13-13: 💤 Low valueAnchor fragility across Tailwind versions
"padding","padding-inline"is positional — if Tailwind ever reorders or renames these adjacent entries, the extractor silently fails with the "not found" error rather than a graceful degradation message. Consider documenting the expected Tailwind version range (or adding a version check) so the failure is obviously actionable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tailwindcss-config-analyzer/src/v4/extract-property-order.ts` at line 13, The hardcoded ANCHOR constant ("padding","padding-inline") is fragile across Tailwind versions; change the extractor to first try a tolerant search for the anchor by locating "padding" and then searching nearby for "padding-inline" (or accept either key independently), fall back to a clear, actionable error that includes the detected Tailwind version or a documented expected version range, and update any code that references ANCHOR (the ANCHOR constant and the extraction logic in extract-property-order.ts) to use the new tolerant lookup and explicit version-check/fallback message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/predicates.rs`:
- Around line 469-490: The is_bg_size function currently ignores invalid
comma-separated segments (e.g., "cover, foo") instead of failing; update the
loop in is_bg_size so that any invalid segment causes an immediate false return:
for each comma-segment (in function is_bg_size), trim and if it's "cover" or
"contain" increment count; otherwise collect tokens into SmallVec as currently
done, and if tokens.len() is not 1 or 2 return false; then if the tokens are all
valid (auto, is_length, or is_percentage) increment count else return false;
keep final return as count > 0.
In `@packages/tailwindcss-config-analyzer/package.json`:
- Line 15: The README/header comment in generate-tailwind-preset-v4.ts and the
package.json script are inconsistent: package.json defines the npm script
"execute:v4" but the file instructs users to run "gen:tailwind-preset-v4";
update one to match the other — either rename the script in package.json to
"gen:tailwind-preset-v4" (keeping the command "tsx
src/v4/generate-tailwind-preset-v4.ts") or change the comment in
generate-tailwind-preset-v4.ts to instruct running "pnpm --filter
tailwindcss-config-analyzer execute:v4", and ensure the chosen name is used
consistently across docs and scripts.
---
Nitpick comments:
In `@crates/biome_js_analyze/tests/sort_v4_test.rs`:
- Around line 13-15: The test currently ignores parse errors from parse_tailwind
and may assert against an error-recovered tree; change the test to include parse
diagnostics in the snapshot output by appending parsed.diagnostics() (or
formatted diagnostics) to the rendered string alongside the existing
parsed.tree()/sorted result, so when calling parse_tailwind and sort_class_list
you render something like the input, parsed.diagnostics(), and sorted to make
parse failures visible during test failures.
In `@packages/tailwindcss-config-analyzer/src/v4/extract-property-order.ts`:
- Line 13: The hardcoded ANCHOR constant ("padding","padding-inline") is fragile
across Tailwind versions; change the extractor to first try a tolerant search
for the anchor by locating "padding" and then searching nearby for
"padding-inline" (or accept either key independently), fall back to a clear,
actionable error that includes the detected Tailwind version or a documented
expected version range, and update any code that references ANCHOR (the ANCHOR
constant and the extraction logic in extract-property-order.ts) to use the new
tolerant lookup and explicit version-check/fallback message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 680776e2-f235-47d8-b40f-787bbb6ff128
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_js_analyze/tests/sort_v4/cases.snapis excluded by!**/*.snapand included by**pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by**
📒 Files selected for processing (17)
crates/biome_js_analyze/Cargo.tomlcrates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes/predicates.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes/tailwind_preset_v4.rscrates/biome_js_analyze/tests/sort_v4/cases.jsonccrates/biome_js_analyze/tests/sort_v4_test.rspackages/tailwindcss-config-analyzer/package.jsonpackages/tailwindcss-config-analyzer/src/v4/css-helpers.tspackages/tailwindcss-config-analyzer/src/v4/extract-property-order.tspackages/tailwindcss-config-analyzer/src/v4/extract-theme-keys.tspackages/tailwindcss-config-analyzer/src/v4/extract-utilities.tspackages/tailwindcss-config-analyzer/src/v4/generate-tailwind-preset-v4.tspackages/tailwindcss-config-analyzer/src/v4/probe-samples.tspackages/tailwindcss-config-analyzer/src/v4/render-rust.tspackages/tailwindcss-config-analyzer/src/v4/theme-namespaces.tspackages/tailwindcss-config-analyzer/src/v4/value-types.ts
…g-size note - Rename the codegen instructions in generate-tailwind-preset-v4.ts and the generated tailwind_preset_v4.rs header to use the actual script name `execute:v4`, matching `package.json`. - Note in `is_bg_size` that the partial-valid acceptance is intentional and mirrors Tailwind v4's `isBackgroundSize`, to forestall reviews that treat it as a bug.
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
This is looking much better overall. Nice job wrangling the agent to avoid heap allocations.
I'm seeing some things that we probably need to improve the parser for. If you could help me extract those requirements, I can work on the parser to get it up to snuff
| "-inset" => FunctionalEntry { | ||
| registration_idx: 0, | ||
| branches: &[ | ||
| Named(ThemeNamespace::Spacing, 4, 1), | ||
| NamedTyped(ValueType::Number, 4, 1), | ||
| NamedTyped(ValueType::Ratio, 4, 1), | ||
| Arbitrary(4, 1), | ||
| ], | ||
| }, | ||
| "inset" => FunctionalEntry { | ||
| registration_idx: 1, | ||
| branches: &[ | ||
| Named(ThemeNamespace::Spacing, 4, 1), | ||
| NamedTyped(ValueType::Number, 4, 1), | ||
| NamedTyped(ValueType::Ratio, 4, 1), | ||
| Arbitrary(4, 1), | ||
| ], | ||
| }, |
There was a problem hiding this comment.
notice how these entries are effectively the same. tailwind doesn't really parse the negative - at the front of candidates. our tailwind parser does.
for example: -inset and inset to be 2 different classes. however, our tailwind parser considers them to be the same inset class, and detects the - as a negative candidate.
There was a problem hiding this comment.
There are two reasons why negative values are assigned separately.
- It allows distinguishing between classes that can be negative and those that cannot. By differentiating that
bg-red-500compiles while-bg-red-500does not, uncompiled items can be moved to the front during sorting. - Most negative classes share the same branch as positive classes, but some differ. The
zclass does not accept aNamedKeyword(e.g.,auto) when negative.bg-linearhas anArbitraybranch when positive but not when negative, and likez, it does not have aNamedKeywordtoo.
"-z" => FunctionalEntry {
registration_idx: 22,
branches: &[
NamedTyped(ValueType::Number, 16, 1),
Arbitrary(16, 1),
],
},
"z" => FunctionalEntry {
registration_idx: 23,
branches: &[
NamedKeyword(0, 16, 1),
NamedTyped(ValueType::Number, 16, 1),
Arbitrary(16, 1),
],
},
"-bg-linear" => FunctionalEntry {
registration_idx: 200,
branches: &[
NamedTyped(ValueType::Number, 196, 3),
NamedTyped(ValueType::Ratio, 196, 3),
ArbitraryTyped(ValueType::Angle, 196, 2),
],
},
"bg-linear" => FunctionalEntry {
registration_idx: 201,
branches: &[
NamedKeyword(12, 196, 3),
NamedTyped(ValueType::Number, 196, 3),
NamedTyped(ValueType::Ratio, 196, 3),
Arbitrary(196, 2),
],
},
Out of 311 Functional Utilities, 70 are negative, and among them, only 10 have a different branch from the positive ones. Therefore, I could omit the duplicates and list only the different classes separately. What do you think is the best way to handle this?
There was a problem hiding this comment.
my main concern is that you have to do string allocations to look up entries in this map, because you have to concatenate the - and the base name.
deduplicating the entries sounds like it would also avoid that problem. so yeah, sounds good.
There was a problem hiding this comment.
I change preset structure to nested. This structure help to avoid string allocation and deduplicate 7b5d63d
| const MATH_FUNCTIONS: &[&str] = &[ | ||
| "calc(", "min(", "max(", "clamp(", "mod(", "rem(", "sin(", "cos(", "tan(", "asin(", "acos(", | ||
| "atan(", "atan2(", "pow(", "sqrt(", "hypot(", "log(", "exp(", "round(", | ||
| ]; | ||
|
|
||
| /// Mirrors <https://github.com/tailwindlabs/tailwindcss/blob/v4.2.2/packages/tailwindcss/src/utils/math-operators.ts#L41> | ||
| /// (`hasMathFn`). | ||
| fn has_math_fn(input: &str) -> bool { | ||
| if !input.contains('(') { | ||
| return false; | ||
| } | ||
| MATH_FUNCTIONS.iter().any(|f| input.contains(f)) | ||
| } |
There was a problem hiding this comment.
to me, this indicates a deficiency in the tailwind parser. what's the context in which these can be used? probably arbitrary values?
There was a problem hiding this comment.
Yes, this function is used by inferring data type at arbitrary value. This is because the CSS-property changes depending on the type of the arbitrary value in some class (e.g. bg-[#abc] -> background-color: #abc / bg-[10px] -> background-size: 10px / bg-[25%] -> background-position: 25%).
+) Since there are often arbitrary fallbacks anyway, it seems like syntax accuracy wasn't a major concern.
There was a problem hiding this comment.
I've opened #10299 to have the parser handle these values more precisely
There was a problem hiding this comment.
I will follow up to simplify logic after the parser change merges!
There was a problem hiding this comment.
Oh yes, exactly what I had in mind.
However I think treating everything as a modifier seems better than adding a Radio token Because Is is possible where Number/Number is treated as a modifier, not a ratio in some case.
text-{font-size}/{line-height} is the case. Default font-size key is string(e.g. lg, xl), but when the user defines a numeral key such as --text-12:12px, then user write class text-12/4. In this case, 12/4 is not ratio token. It is font-size: 12px; line-heigt:1rem
Therefore how about not adding ratio token, just treating modifier? If NamedTyped branch has ratio case, determine toke is valid by check modifier and value like tailwind.
There was a problem hiding this comment.
oof yeah i was kinda on the fence about the ratio type. I agree. I think we'll need it eventually (because it is a distinct type in tailwind's semantics), but for now I'll get rid of it.
There was a problem hiding this comment.
Yeah, If the PR merged, I really will remove all about predicate code!
There was a problem hiding this comment.
I deleted predicate code and update code to using parsed value
…cates Replace the eight `LazyLock<Regex>` statics with byte-level scanners (number / percentage / ratio / length / angle / vector) and prefix-array matchers (gradient / image / color functions). Behaviour stays a 1:1 mirror of Tailwind v4.2.2's `infer-data-type` patterns; the existing 30 unit tests cover the edge cases. This addresses the reviewer note that regex usage is discouraged in this crate's hot paths.
Match what IDE folding (rust-analyzer / VSCode) recognises so the predicate file's logical sections collapse cleanly. Address review nit on predicates.rs section markers.
Cover the three internal contracts that the snapshot fixture cannot isolate: - compare ordering rules (Unknown < Known, property_idx asc, property_count desc, registration_idx asc, equal-key invariance). - resolve_branch dispatch (first match wins, ArbitraryTyped/Arbitrary skipped for bare values, registration_idx threading, no-match path). - sort_class_list edge cases (empty input, whitespace-only). Tests are grouped under collapsible region/endregion markers.
Looking up `-<basename>` by reconstructing the dashed key would force a
heap allocation per token; merging negatives into the positive entry
keeps the lookup at a single alloc-free phf hit.
Functional side gains an `Option<Negative>`:
- `None`: utility rejects negation (`bg`, `flex`, ...).
- `Some(SameBranches { registration_idx })`: 60 of 70 cases — the
negative form reuses the positive's branch list and only needs its
own sort tie-break index.
- `Some(Distinct { registration_idx, branches })`: the 10 cases that
diverge structurally (`-z`/`-order`/... drop the `auto`/`first`
NamedKeyword; `-bg-linear` keeps only `ArbitraryTyped(Angle)`).
Static side gains `negative_registration_idx: Option<u16>` for the
sign-paired statics (`inset-px`/`-inset-px`, ...).
…ndidates Drop the early-skip on `negative_token` and route through the new preset metadata: - Functional candidates match `entry.negative` to pick the correct (registration_idx, branches) slot, falling back to `Unknown` when the utility rejects negation (`-bg-red-500`, `-text-lg`) or when the Distinct branch list no longer accepts the value (`-z-auto`). - Static candidates use `entry.negative_registration_idx` to keep the negative form's own tie-break key. `None` reduces to `Unknown` — exercised by `-flex` (Tailwind never registers it). Fixture grows by 7 cases that intentionally jumble input order so the new sort paths are observable in the snapshot.
| const MATH_FUNCTIONS: &[&str] = &[ | ||
| "calc(", "min(", "max(", "clamp(", "mod(", "rem(", "sin(", "cos(", "tan(", "asin(", "acos(", | ||
| "atan(", "atan2(", "pow(", "sqrt(", "hypot(", "log(", "exp(", "round(", | ||
| ]; | ||
|
|
||
| /// Mirrors <https://github.com/tailwindlabs/tailwindcss/blob/v4.2.2/packages/tailwindcss/src/utils/math-operators.ts#L41> | ||
| /// (`hasMathFn`). | ||
| fn has_math_fn(input: &str) -> bool { | ||
| if !input.contains('(') { | ||
| return false; | ||
| } | ||
| MATH_FUNCTIONS.iter().any(|f| input.contains(f)) | ||
| } |
There was a problem hiding this comment.
Yeah, I thought that's what you needed. Can you elaborate what you mean by predicates?
jiwon79
force-pushed
the
feat/tailwind-v4-utility-sort
branch
from
d87b4a6 to
2584190
Compare
NamedTyped only carries Number/Percentage/Ratio in the preset, so delete the 12 dormant Color/Length/Angle/Image/etc. predicates and their helpers. ValueType::matches now dispatches only those three and returns false for the rest. ArbitraryTyped/Arbitrary branches stay in the preset for the follow-up PR that wires them up via the parser nodes from biomejs#10299.
Routes sort_v4 through the new TwNumberValue / TwPercentageValue nodes from biomejs#10332 instead of scanning value text in predicates.rs. Ratio is synthesized from value + numeric modifier when the entry has a Ratio branch (mirrors Tailwind's supportsFractions). All hot-path matching stays allocation-free.
github-actions
Bot
added
A-CLI
jiwon79
force-pushed
the
feat/tailwind-v4-utility-sort
branch
from
ccbccc5 to
84d2dec
Compare
github-actions
Bot
removed
A-Project
|
Can I get a review? @dyc3 |
2 participants
Summary
Related to #1274.
Sort Module
Adds an internal sort module sort_v4 that using biome_tailwind_parser instead of the existing class lexer.
Current logic cover only pure utilities and negative value. (e.g. flex, bg-red-500, -mt-2)
Out of scope (each tagged with a // TODO: comment in sort_v4.rs and it will be covered on next PR):
Codegen
Tailwind v4 sort standard is complied css property from classname unlike v3 standard is only tailwind classname. A new TS codegen extracted the v4 preset, which is the information needed to match tailwind class to css-properties. (PROPERTY_ORDER, STATIC_UTILITIES, FUNCTIONAL_UTILITIES with Branch list, theme-namespace key sets)
The 17 v4 value-type predicates (is_color, is_length, is_ratio, …) are ported 1:1 from Tailwind's infer-data-type.ts into predicates.rs, each with a source-link comment.
Test Plan
crates/biome_js_analyze/tests/sort_v4_test.rsruns every entry intests/sort_v4/cases.jsoncpredicates.rscover Color / Length / Ratio / Position / BgSize / LineWidth / VectorDocs