-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Expose candidatesToAst to the language server
#19405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This will be used to help speed up AST conversions significantly
WalkthroughThe design-system module now exposes a new public method Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (3)packages/tailwindcss/src/design-system.ts (4)
packages/tailwindcss/src/index.ts (1)
packages/tailwindcss/src/intellisense.test.ts (1)
⏰ 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)
🔇 Additional comments (12)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tailwindcss/src/design-system.ts (1)
114-151: candidatesToAst/candidatesToCss behavior aligns with the compilation pipeline; consider deduplicating src-propagation logicThe per-class
compileCandidatescall, invalid-candidate guard (wasValid+ empty array), optionalutilitiesNode.srcpropagation viawalk, and use ofoptimizeAst(…, Polyfills.None)give you a clean, source-aware AST tailored for IntelliSense, and reusing it incandidatesToCssavoids double work. One small polish opportunity: thewalk(…, node.src ??= utilitiesNode.src)pattern is now duplicated here and incompileAst.buildinindex.ts; factoring this into a shared helper (e.g.,attachUtilitySources(astNodes, utilitiesNode)) would keep source-location behavior in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/tailwindcss/src/design-system.ts(5 hunks)packages/tailwindcss/src/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/tailwindcss/src/index.ts (1)
packages/tailwindcss/src/design-system.ts (1)
buildDesignSystem(69-251)
packages/tailwindcss/src/design-system.ts (3)
packages/tailwindcss/src/ast.ts (3)
AstNode(68-68)optimizeAst(220-672)toCss(674-879)packages/tailwindcss/src/walk.ts (1)
WalkAction(10-20)packages/tailwindcss/src/value-parser.ts (1)
toCss(41-56)
🔇 Additional comments (4)
packages/tailwindcss/src/design-system.ts (2)
2-2: Imports for AST typing and walking are appropriatePulling in
type AstNodefrom./astandWalkAction/walkfrom./walkmatches the new usage incandidatesToAstand keeps types explicit without affecting runtime behavior. No issues here.Also applies to: 27-27
61-67: DesignSystem API extension and buildDesignSystem signature look correctAdding
candidatesToAst(classes: string[]): AstNode[][]toDesignSystemand wiring it (plus the refactoredcandidatesToCss) into the returneddesignSystemobject is consistent with the stated IntelliSense use case. MakingutilitiesNodean optional parameter onbuildDesignSystemkeeps the call sites backwards compatible while enabling source-aware behavior when it is provided.Also applies to: 69-72, 160-162
packages/tailwindcss/src/index.ts (2)
598-605: Passing utilitiesNode into buildDesignSystem is consistent with the new AST APIsFeeding
utilitiesNodeintobuildDesignSystem(theme, utilitiesNode)nicely connects the parse phase to the newcandidatesToAstlogic that reusesutilitiesNode.srcfor generated utility nodes. This keeps the source-location handling centralized in the design system without changing behavior for callers that don’t use@tailwind utilities(whereutilitiesNodeis null).
829-844: Using CSS.parse with{ from: opts.from }correctly enables source tracking for both compile and __unstable__loadDesignSystemParsing with
CSS.parse(css, { from: opts.from })in bothcompileand__unstable__loadDesignSystemaligns the AST’ssrcinformation with the caller-provided filename and matches the later use oftoCss(newAst, !!opts.from)andutilitiesNode.srcin downstream code. This is a straightforward, backwards-compatible improvement for source maps and language-server consumers.Also applies to: 858-859
a4cee91 to
5a36b3c
Compare
Companion PR to tailwindlabs/tailwindcss#19405 The aim of this PR is to improve the performance around looking up / compiling candidates into utilities when using v4. It does this in three stages: 1. For earlier versions (v4.0–v4.1.17) we swap out PostCSS (mostly) with our own CSS parser. The implementation is tailored to improve performance and memory usage for how we use it. This also means I've been able to optimize how we walk the AST and remove mutations in many places. There *are* still some internal APIs that use PostCSS because they interface with multiple versions of Tailwind CSS. These still use PostCSS and we translate our AST into a PostCSS AST for these cases. Eventually these APIs will also be converted to use our internal CSS AST directly — we'll support old versions by converting *from PostCSS* to our internal AST — that is for a future PR. 2. For new enough versions, since our AST here mirrors the one in Tailwind CSS, we can use the nodes directly from Tailwind CSS itself instead of generating the code inside Tailwind CSS, serializing to a string, and then finally re-parsing. This is exactly what the `candidatesToAst` API enables (will ship in Tailwind CSS v4.1.18). 3. A large portion of the candidate compiling perf is spent on color lookups. The above will help with that significantly but there's still a large amount of time analyzing declaration values for colors. This PR replaces our unwieldy color regex to pick out potential colors with a fine tuned parser. This speedup gives us a good bit more headroom for re-enabling mask utility color swatches. The overhead from parsing an extra 5k utilities is still too significant but it is now *much* lower than before. The next big improvements will need to happen in core.
This will be used to improve performance and potentially enable future features that require generated CSS source locations.
Note: This is still 100% internal API. You can only access this via
__unstable__loadDesignSystemfor a reason. We may chance the structure of the arguments and/or return values as needed.