feat: allow using @variant with multiple comma-separated variants#19526
feat: allow using @variant with multiple comma-separated variants#19526orteth01 wants to merge 6 commits intotailwindlabs:mainfrom
Conversation
WalkthroughThe pull request adds support for comma-separated variants inside 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tailwindcss/src/variants.ts (1)
1215-1231: Consider cloning nodes for each variant iteration to prevent potential shared-state issues.Each iteration creates a new
styleRulewith the samevariantNode.nodesreference. While variant functions typically reassignr.nodesrather than mutating in place, the underlying AST nodes (e.g., declaration objects) are shared across iterations. If any downstream processing mutates these objects in place, all variants would be affected.For defensive coding, consider cloning the nodes for each iteration using the already-imported
cloneAstNode:🔎 Suggested defensive change
for (let variant of variants) { // Starting with the `&` rule node - let node = styleRule('&', variantNode.nodes) + let node = styleRule('&', variantNode.nodes.map(cloneAstNode))
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/tailwindcss/src/index.test.tspackages/tailwindcss/src/variants.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/tailwindcss/src/index.test.ts (2)
packages/tailwindcss/src/test-utils/run.ts (1)
compileCss(4-11)integrations/utils.ts (1)
css(519-519)
🔇 Additional comments (1)
packages/tailwindcss/src/index.test.ts (1)
5302-5332: Test coverage looks good for the primary use case.The test correctly verifies that comma-separated variants produce independent rule sets with proper variant-specific behavior (hover wrapped in
@media (hover: hover), focus applied directly).Consider adding edge case tests in a follow-up for robustness:
- Three or more variants:
@variant hover, focus, active { ... }- Whitespace variations:
@variant hover,focus { ... }vs@variant hover , focus { ... }- Nested comma-separated variants
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tailwindcss/src/index.test.ts (1)
5337-5509: Excellent test coverage for comma-separated variant functionality!The new test suite comprehensively validates the comma-separated
@variantfeature with tests covering basic usage, multiple variants, whitespace handling, and nesting. The test structure is consistent with existing patterns and the snapshots confirm correct behavior.Optional enhancement: Consider adding edge case tests for invalid inputs to ensure graceful handling:
Suggested additional test cases
it('should handle trailing commas gracefully', async () => { await expect( compileCss( css` .btn { background: black; @variant hover, focus, { background: red; } } @tailwind utilities; `, [], ), ).resolves.toBeDefined() // or .rejects if it should error }) it('should handle empty variant names', async () => { await expect( compileCss( css` .btn { @variant hover, , focus { background: red; } } `, [], ), ).resolves.toBeDefined() // or .rejects if it should error })These additional tests would help ensure the parser handles malformed input predictably, but the current coverage is sufficient for the main feature.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/tailwindcss/src/index.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/tailwindcss/src/index.test.ts (2)
packages/tailwindcss/src/test-utils/run.ts (1)
compileCss(4-11)integrations/utils.ts (1)
css(519-519)
discussion: #19319
Summary
in css, now can apply the same styles to multiple variants in one place. e.g.
Test plan
@variantwith multiple, comma-separated variants