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 into
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 |
![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/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)
orteth01
changed the title
|
Hey! Thanks for the PR, I implemented this as part of #19996 but your commits are still included so you are still marked as a contributor. |
…indlabs#19996) This PR improves and simplifies the `@variant` usage. When we originally added support for `@variant`, we wanted to keep things simple, where we could only use a single variant at a time. The original PR did have a more complex system with all these features enabled, but we wanted to make sure that we only introduced the additional complexity when the community felt like it was needed. But of course we still wanted to make sure that you could do compound and stacked variants, it just required some additional code. For compound variants, where you want to use variant `a` and variant `b`, you could duplicate the rules as siblings: ```css .foo { @variant a { display: flex; } @variant b { display: flex; } } ``` But with this PR, you can comma separate each variant to get the same effect: ```css .foo { @variant a, b { display: flex; } } ``` You can think of this as-if we are expanding this syntax into the aforementioned syntax. In other words, we would do the duplication for you. Additionally, you also want to be able to stack variants. For that you had to nest your `@variant` rules: ```css .foo { @variant a { @variant b { display: flex; } } } ``` Not the end of the world, but it can get pretty nested if you want to use multiple variants. Luckily we already have a syntax for this in normal Tailwind CSS classes: `a:b:flex`. Which is exactly what we can use here as well: ```css .foo { @variant a:b { display: flex; } } ``` Again, conceptually you can think of this syntax being expanded into the syntax from above. Last but not least, we can also combine these: ```css .foo { background: black; @variant a, b:c { background: red; @variant d, e:f { background: blue; } } } ``` This conceptually translates into the much more verbose version today: ```css .foo { background: black; @variant a { background: red; @variant d { background: blue; } @variant e { @variant f { background: blue; } } } @variant b { @variant c { background: red; @variant d { background: blue; } @variant e { @variant f { background: blue; } } } } } ``` The biggest downside is that this could potentially easily balloon your CSS file size if you're not careful. Because with this, it's pretty easy to add one more variant that introduces a lot of duplicated CSS. This feature is completely backwards compatible, you can still nest your `@variant` calls yourself if you want, and combine them with these features if you want. This is also a continuation of tailwindlabs#19526 and tailwindlabs#19884, but for some reason I don't have push rights, so I'm creating this new PR instead. I did keep the original commits of those PRs so these contributors are still properly marked as contributors. <img width="808" height="135" alt="image" src="https://github.com/user-attachments/assets/bee334ab-39d7-4d4d-a48f-afa2253cf17b" /> <img width="349" height="75" alt="image" src="https://github.com/user-attachments/assets/fb68906c-db74-43f7-83e4-918ad3d4a036" /> Closes: tailwindlabs#19526 Closes: tailwindlabs#19884 ## Test plan 1. Added a bunch of new tests to verify this new behavior 2. Added tests that compare the short (new) version, and the long (old) version 3. Added a sourcemap related test to ensure that the src and dst locations are correct 4. Existing tests still pass --------- Co-authored-by: orteth01 <tortega128@gmail.com> Co-authored-by: Ray Knight <array.knight+github@gmail.com>
2 participants
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