Handle CSS nesting natively#20124
Conversation
d7aaac0 to
9e82582
Compare
Use the `pretty(…)` helper for the AST related tests wherever we didn't do that yet.
This is temporary and on purpose. Once we implement nesting, we can see the impact on the tests. Once we are happy with that, we can add the optimizations from Lightning CSS again and if everything goes well, we should have barely any CSS changes in the final diff (at least related to CSS nesting).
Will be undone once Lightning CSS is enabled again
Will be re-enabled once Lightning CSS is enabled again
These tests don't really test AST specific things. They really test how `walk` works and what happens when you use certain `WalkAction` commands...
This part of the optimize AST step is covered automatically by the custom nesting handling.
402c33e to
2efc0c1
Compare
fix crash when using `@layer x; @layer x {…}` We were reading from `lastNode` that could be empty because of the body-less `@layer x` ensure we don't loose information when merging `@font-face` `@font-face` does't have params, but the body can be different. We should _not_ be merging these otherwise we might loose information. do not omit `*` when it's a namespace scope selector parser cache to `handleNesting`
2efc0c1 to
aa89e73
Compare
Confidence Score: 5/5This looks safe to merge.
Reviews (4): Last reviewed commit: "swap checks, only compute `path` when ab..." | Re-trigger Greptile |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChangesThis PR adds Related PRs: None identified. Suggested labels: area: core, area: tests, needs-changeset Suggested reviewers: thecrypticace, RobinMalfait 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/tailwindcss/src/utils/variables.ts (1)
3-23: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueModule-level cache never evicts and returns a shared mutable array.
DefaultMaphas no eviction (confirmed: it's a plainMapsubclass that permanently stores factory results), and this cache is module-scoped rather than scoped per-compile like the analogousparseSelectorCacheinast.ts'shandleNesting. In practice this is bounded today sinceextractUsedVariablesis only invoked for@themevariable declaration values (a small, stable set per project), so the growth risk is low. Also,.get(raw)returns the same array instance to every caller — currently safe since callers only iterate read-only, but a future caller that mutates the returned array would silently corrupt the cache for all other callers.Consider scoping the cache per-
optimizeAstinvocation (matching theparseSelectorCachepattern) or returning a defensive copy, to avoid unbounded growth in long-running watch/dev-server processes and to guard against future mutation bugs.♻️ Optional: return a copy instead of the cached reference
export function extractUsedVariables(raw: string): string[] { - return extractUsedVariablesCache.get(raw) + return extractUsedVariablesCache.get(raw).slice() }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9be5ee53-8fa8-4cab-a86f-d287a5658cf5
📒 Files selected for processing (18)
integrations/cli/plugins.test.tsintegrations/vite/index.test.tsintegrations/vite/resolvers.test.tspackages/tailwindcss/src/ast.test.tspackages/tailwindcss/src/ast.tspackages/tailwindcss/src/attribute-selector-parser.test.tspackages/tailwindcss/src/attribute-selector-parser.tspackages/tailwindcss/src/compat/config.test.tspackages/tailwindcss/src/compat/plugin-api.test.tspackages/tailwindcss/src/compat/plugin-api.tspackages/tailwindcss/src/intellisense.test.tspackages/tailwindcss/src/selector-parser.test.tspackages/tailwindcss/src/selector-parser.tspackages/tailwindcss/src/source-maps/source-map.test.tspackages/tailwindcss/src/utilities.test.tspackages/tailwindcss/src/utils/variables.tspackages/tailwindcss/src/variants.test.tspackages/tailwindcss/src/variants.ts
…ctor inside of a complex selector
handle non-ascii characters during attribute parsing
If you are using `@variant` in your CSS without any rules, then we could potentially introduce invalid CSS. We start from a rule with a `&` selector, but if we didn't have a selector in the first place then we would introduce a selector which is incorrect. This fix tracks the nodes of the intermediate node if that's the case.
We were incorrectly mapping the source locations of rules to that of the used declaration which was wrong.
We are not too strict about the syntax, but the grammar looks like:
```
:nth-child([ <An+B> | even | odd ] [of <complex-selector-list>]?) {
/* ... */
}
```
Which means that you could write `:nth-child(2 of &)` and we have to
repalce the `&` with the parent selector.
parse out selector list in `:nth-child` and `:nth-last-child`
aa89e73 to
dc1758a
Compare
We are checking wether the to-be-replaced ast is a complex selector or not. If it is, only then do we care about the type of the grand parent selector.
e2d3139 to
9107de5
Compare
This PR introduces a new feature where we will be handling the CSS nesting ourselves.
We currently still rely on Lightning CSS in most places. But there are situations where we don't use Lightning CSS out of the box:
@tailwindcss/browseror in Tailwind Play.We handle CSS nesting in a single pass over the AST by tracking some information as we go. It's not the most complex code, but there are some tricky parts to make this happen in an efficient way, especially for the few additional optimizations we handle.
While going over the AST, we will only emit CSS the moment we see declarations or comments. This also means that this has a fun side effect of removing CSS that ends up with empty nodes automatically. (Caveat: there are exceptions for body-less rules such as
@layer foo;or `@charset "UTF-8";)This also allowed us to do some cleanup in
optimizeAstthat tried to do this as well, but now this will be handled by the code that handles nesting automatically. Which is preferred because the version inoptimizeAstmutated the AST.This also contains some optimizations where we merge adjacent at-rules (with the same name / params), and adjacent rules with the same selector, and get rid of declarations that are duplicated in a node. (Caveat: there are exceptions, in case of
@font-family { … }where we don't want to merge them)To ensure that this implementation is correct, I also added an oracle implementation in the tests. This implementation does multiple passes over the AST, because it does each step one by one, with minimal code. Each step contains comments with examples to see what's happening in that step. We then test the optimized version against this.Once the implementation was in place, and all the tests were passing, then I deleted the oracle implementation. That way we don't have to keep it in sync all the time.While handling the nesting, we have to make sure that
&exists and if we replace it with a parent selector that we do use:is(…)semantics. This means that:Becomes:
We then also make sure that we optimize the selector by removing the unnecessary
:is(…)wrappers, but only if they were introduced by the nesting logic. If you wrote:is(…)in your CSS, we won't touch it.If you look at the commits, the first thing we did is remove the optimization step from Lightning CSS in the tests. Then we enabled our CSS nesting handling code. This allows us to see the effect of the changes we are making. At the end, we re-enabled Lightning CSS.
For now, this PR will be a step that happens before Lightning CSS is executed, while still using Lightning CSS. But now this step will also always happen in places where we don't use Lightning CSS at all.
This should not result in any breaking changes. It could result in changed CSS output in environments where Lightning CSS isn't used. In environments where it is being used, then there could be some differences related to some selectors but they should result in the same behavior with the same specificity.
While testing things, I noticed that there are some missed opportunities for performance related to how we extract variables from declaration values. I want to tackle
optimizeAstin future PRs to make it simpler, more performant, and maybe even merge it with the CSS nesting handling.As part of testing this, I tested it against the tailwindcss.com codebase which contains a lot of CSS (807.67 KB, 18 174 AST nodes) because almost every utility is being used in examples.
The oracle implementation is rather slow:
But the final code is much faster (
<15ms):In contrast, Lightning CSS takes:
[ 37.48ms] Optimized by Lightning CSSOne interesting thing to notice is that in big projects, this could add
10msto the build, but Lightning CSS would then take less time to process, which results in a no-op with better output.One thing to keep in mind here is that Lightning CSS does more things, such as normalizing values, handling vendor prefixes, CSS nesting, etc.
Some notes on how the algorithm works:
The basic idea
When you have CSS that looks this:
Then the AST looks like this:
When we walk this tree, and we encounter a
rule, then we will track the selector on a stack. When we are done walking over the rule, then we will pop the selector from the stack. This means that the top-most selector on the stack will always be the parent selector.The moment we encounter a
rule, and if a previous rule was seen, then we push theselectorof the rule onto the stack, but in a way that the&is already replaced by the selector. This way, a sibling rule will also get the already-prepared parent selector.The simple version looks like this:
So far we aren't doing much yet, but the interesting part is when we encounter a
declaration(or acomment). The moment we see any of those, then will we emit a node with the information from theselectorStack.We then also track the last node's
nodeswe created such that we can push more declarations into it as a shortcut.The last important part is that whenever we see a new
rule, then we have to reset thatnodestracking variable such that we can create a fresh node the next time we see a declaration.For the
at-rules, something similar happens but they are tracked in a similar but separate stack. The idea there is that we can then wrap thoseat-rulesaround thenewNodewe create. That way the at-rules naturally float to the top.I can keep going here, but I think if you're interested in this, then you could go over the commits in this PR, or you can look at the
ast.tsimplementation directly to see what's going on.Test plan