Conversation
WalkthroughThis change aligns Tailwind's Pre-merge checks✅ Passed checks (4 passed)
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: 0
🧹 Nitpick comments (1)
packages/tailwindcss/src/utilities.test.ts (1)
27211-27234: Consider adding explicit test cases for multiple slashes (core issue #19505).The test suite covers many edge cases well, but given that issue #19505 specifically concerns multiple slashes in utility names (e.g.,
ui/button/sm), consider adding explicit test cases:🔎 Suggested additions for slash handling clarity
['foo-4/snug', true], // Bare value with named modifier ['foo_', true], // This is supported today, so let's not break it + ['foo/bar', true], // Single slash as modifier separator ['Foo', false], // Starting with uppercase letter is invalid ['-Foo', false], // Starting with uppercase letter is invalid (negative) ['foo-', false], // Should not end with a dash ['foo-1/', false], // Invalid fraction/modifier + ['foo/bar/baz', false], // Multiple slashes are invalid (issue #19505) ['foo-p%', false], // Invalid percentageThis would serve as a clear regression test for the specific issue being fixed and document the expected single vs. multiple slash behavior directly in the validation function's test suite.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.mdpackages/tailwindcss/src/index.test.tspackages/tailwindcss/src/utilities.test.tspackages/tailwindcss/src/utilities.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/tailwindcss/src/index.test.ts (2)
integrations/utils.ts (2)
test(81-456)css(519-519)packages/tailwindcss/src/test-utils/run.ts (1)
compileCss(4-11)
🔇 Additional comments (6)
CHANGELOG.md (1)
16-16: Changelog entry for@utilityvalidation looks accurate and consistentThe wording clearly describes the behavior change, matches the surrounding style in the “Fixed” section, and correctly links to the PR. No further adjustments needed.
packages/tailwindcss/src/index.test.ts (1)
4663-4697: LGTM! Comprehensive test for utility name validation.This test effectively validates the new
@utilityname constraints introduced to fix issue #19505:
- Positive case: Correctly verifies that a single
/(modifier separator) inui/buttonis valid and generates properly escaped CSS- Negative case: Correctly verifies that multiple
/characters inui/button/smtrigger a validation errorThe test structure is excellent with clear inline snapshots documenting the expected behavior for both valid and invalid inputs. This ensures the Oxide scanner rules are consistently enforced at the core level.
packages/tailwindcss/src/utilities.ts (2)
6429-6568: LGTM! Excellent implementation of utility name validation.The validation logic correctly enforces all the rules described in the PR objectives:
✅ Valid root pattern: optional
-, lowercase letter start, then[a-zA-Z0-9_-]*
✅ Single slash only as modifier separator, must be followed by characters
✅ Dots only between digits, prevents consecutive dots
✅ Percent only at end when preceded by digit
✅ Character-by-character validation for static utilities
✅ Functional utilities correctly reject characters between root and-*suffixThe use of character codes for performance is well-balanced with readability through clear constant names and comprehensive comments. Edge cases like empty strings, trailing dashes, and invalid character positions are all properly handled.
5835-5835: LGTM! Validation functions correctly integrated.The new validation functions are properly used at the call sites where
@utilitynames need to be validated:
- Line 5835: Functional utilities (
*suffix)- Line 6184: Static utilities
This successfully moves validation from the Oxide scanner into Tailwind core as intended.
Also applies to: 6184-6184
packages/tailwindcss/src/utilities.test.ts (2)
4-4: LGTM!The import correctly brings in the two new validation functions that are being tested.
27236-27249: LGTM!The functional utility name tests comprehensively cover the validation rules: the
-*suffix requirement, negative prefixes, uppercase restrictions, and special character rejection.
- We check for a valid root which should optionally start with `-`, and
be followed by `[a-z]`. After that, only `[a-zA-Z0-9_-]` is valid for
the root.
- For static utilities, the remaining part (the value) can include:
- `.`: but not consecutive ones, e.g. `foo..bar` is invalid
- `%`: but only at the end and preceded by a digit, e.g. `foo-x%` and
`foo-%-bar` are invalid
- `/`: as the modifier, but there can only be one, and must be
followed by another character, e.g.: `foo/bar/baz` and `foo/` are
invalid
- For functional utilities, we need a valid root and a valid `-*`
suffix. The remaining "value" part should be empty.
These are the ones that represent the linked issue even though we have a dedicated regrression test for it.
RobinMalfait
force-pushed
the
fix/issue-19505
branch
from
908111c to
2a9a4f7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tailwindcss/src/utilities.test.ts (1)
27211-27251: Good test coverage for the new validation functions.The test cases comprehensively cover the validation rules for both static and functional utility names, including the key case
foo/bar/bazthat addresses issue #19505. The use oftest.eachkeeps the tests readable and maintainable.Minor observation: The test descriptions (
'valid static utility name "%s" (%s)') say "valid" but also test invalid cases. Consider rewording to something like'isValidStaticUtilityName("%s") should be %s'for clarity, though this is purely stylistic.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.mdpackages/tailwindcss/src/index.test.tspackages/tailwindcss/src/utilities.test.tspackages/tailwindcss/src/utilities.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/tailwindcss/src/index.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/tailwindcss/src/utilities.test.ts (1)
packages/tailwindcss/src/utilities.ts (2)
isValidStaticUtilityName(6443-6530)isValidFunctionalUtilityName(6532-6568)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Linux
- GitHub Check: Linux / upgrade
- GitHub Check: Linux / postcss
- GitHub Check: Linux / cli
- GitHub Check: Linux / vite
🔇 Additional comments (6)
CHANGELOG.md (1)
17-17: Changelog entry is consistent and clearThe new bullet follows the existing style (imperative verb, scoped to
@utility, includes PR link) and accurately describes the change in validation behavior.packages/tailwindcss/src/utilities.test.ts (1)
4-4: LGTM!Import correctly added for the validation functions under test.
packages/tailwindcss/src/utilities.ts (4)
6429-6442: LGTM! Clean constant definitions for validation logic.The
UTILITY_ROOTpattern correctly enforces that utility names must start with a lowercase letter (after an optional leading dash), which aligns with the Oxide scanner rules and properly rejects invalid names likeFooor-Foomentioned in the PR objectives.
6443-6530: LGTM! Comprehensive validation logic correctly implements Oxide scanner rules.The function properly handles all the edge cases mentioned in the PR objectives:
- ✅ Rejects multiple slashes:
foo/bar/baz- ✅ Rejects trailing slash:
foo-1/- ✅ Rejects trailing dash:
foo-- ✅ Rejects dots not between digits:
foo.bar,foo-1..5- ✅ Allows valid patterns:
foo-1.5,foo-50%,foo-1/2The character-by-character validation with proper state tracking (e.g.,
seenSlash) is efficient and correct.
6532-6568: LGTM! Functional utility name validation is correct.The function properly validates functional utilities (ending with
-*) and correctly rejects invalid patterns:
- ✅ Accepts simple functional utilities:
tab-size-*- ✅ Rejects double-dash before suffix:
tab-size--*- ✅ Rejects additional characters before suffix:
tab-size-[…]-*The validation ensures functional utilities maintain a clean
<root>-*pattern only.
5835-5835: LGTM! Validation functions properly integrated into utility creation.The new validation functions are correctly invoked in
createCssUtilityto ensure all custom@utilitydefinitions are validated consistently across environments (Oxide scanner, CLI/Vite, CDN, Tailwind Play), which addresses the core objective of fixing issue #19505.Also applies to: 6184-6184
## Problem Tailwind 4.2.0 introduced stricter `@utility` name validation (#19524) that rejects functional utility names where the root ends with a dash after stripping the `-*` suffix. This breaks a valid and useful naming pattern where a double dash separates the CSS property from a value scale: ```css @Utility border--* { border-color: --value(--color-border-*, [color]); } ``` This produces: `border--0`, `border--1`, `border--2`, etc. The error message is: > `@utility border--*` defines an invalid utility name. Utilities should be alphanumeric and start with a lowercase letter. ## Why this pattern matters The double-dash convention creates a clear visual grammar in class names. The first segment names the CSS property, and the double dash separates it from the semantic scale value. In a dense className string like `border border--0 background--0 content--4`, the scale values (0, 0, 4) are immediately scannable, distinct from the single-dash property names around them. This pattern is actively used in production design systems for semantic color scales (background, content, border, shadow) with values from 0-10. ## Why the restriction is unnecessary The validation comment states the concern is that `border--*` could match the bare class `border-` when using default values. However, this edge case is already handled: 1. **`findRoots` in `candidate.ts`** (line 887) already rejects empty values: `if (root[1] === '') break` 2. **The Oxide scanner** already extracts double-dash candidates correctly, as confirmed by existing tests: `("items--center", vec!["items--center"])` The candidate parser and scanner both handle this case. The validation was an overcorrection. ## Changes - Removed the trailing-dash check from `isValidFunctionalUtilityName` in `utilities.ts` - Updated the existing unit test from `['foo--*', false]` to `['foo--*', true]` - Added an integration test proving `@utility border--*` compiles correctly with theme values ## Test results All 4121 tests pass across the tailwindcss package, including the new integration test. --------- Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
1 participant
This PR improves the validation of allowed
@utility …names.Each
@utilityname should be a valid Tailwind CSS class, so new syntaxes should not be allowed, e.g.foo/bar/bazwould be invalid.We already enforce this behavior but not consistently. The Oxide scanner that scans all your source files for potential Tailwind CSS classes does enforce all of these rules already. So if you used
@utility foo/bar/baz {}, the Oxide scanner would not pick upfoo/bar/bazas a valid class name, so for that reason it's not a breaking change.Where we didn't enforce it is in places where you use the development-only CDN or Tailwind Play. That's because those environments don't use the Oxide at all, and get the classes from the DOM directly and pass it to Tailwind's compiler.
This PR moves some of these validation rules into Tailwind's core when defining custom
@utilityutilities.Fixes: #19505
Test plan
@utilitynamesI also confirmed with Oxide to know which classes were actually valid and which ones are invalid.
Given this input CSS:
And this HTML:
Then all classes in the
Extractedsection are found. One funny thing in the not extracted section is that thebarinfoo.barandfoo..baris also extracted. Feels like a potential bug, but out of scope for this PR.