Repository navigation
Extract CSS in a single pass over the declarations - #105
Merged
Merged
Conversation
Static resolution now takes values from `const` bindings only. `let`/`var`
bindings, parameters and destructured bindings are still recorded so they
shadow outer bindings and imports (including `var` hoisting), but a
template using one is skipped with UNRESOLVED_INTERPOLATION: a `let`/`var`
may be reassigned and a `var` is `undefined` until its declaration has run,
which static analysis cannot follow faithfully. A css`` template assigned
to a `let`/`var` is still extracted; only references to it need a `const`.
Templates are collected during the visit and extracted once every scope is
populated, so a local binding shadowing the tag — even one declared later
and hoisted — leaves the template alone; the tag is recognized through its
import binding, which also supports `import { css as styled }`.
Scopes: function bodies are scopes of their own (parameter defaults cannot
see the body, body `var`s hoist into the body), loop bodies are separate
from their heads, the switch scope starts with the first case, the `var`
declarator kind is tracked as a stack, and a template's interpolations
resolve where the template appears rather than in the `var` target scope.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Conflicts: - src/index.ts: keep the PR's loop, switch, and tag-shadowing scopes with main's JSDoc and `ecij()` signature - AGENTS.md: main's transform description with the PR's resolution rules - test fixtures and snapshots: main's formatting and IIFE class names; the barrel test now records the warning from scoping.input.ts's `var` case Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop "e.g."/"i.e." and README semicolons, unwrap the README section, and use JSDoc for interface fields like `Scope`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Drop `CssTagCandidate.bindingScope`: only a `const` records its class name, and a `const` is bound in the scope of its template. - Give catch bodies their own nested scope, like loop bodies, which retires `skippedBlockStatements`. - Share `resolveStaticExpression` between interpolations and `const` initializers, so `const offset = -5` resolves, as the README states. - Require the scope argument of `recordIdentifierWithValue`: templates are extracted after the visit, when `currentScope` is the root again. - README: drop the ambiguous "or a class" and the repeated list of bindings that are not resolved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The first pass used to record the class names of the templates without interpolations, so that the second pass could resolve them. parseFile records every class name now, so the two passes only reordered the extractions, which a sort then put back in source order. The declarations are already in source order, so a single loop produces the same output without the sort, the `hasInterpolations` flag, or a separate array of replacements. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
A `const` value is now evaluated once every scope is populated, like the tag shadowing, so an initializer may also reference another binding: `const accent = brand` takes the value of `brand`, class names included, following lexical scoping. The values are evaluated in rounds, as an initializer may reference a `const` evaluated after it, until a round makes no progress, which leaves cycles unknown. An alias of an import is not followed. `isBoundInScopeChain` becomes `findBindingScope`, which `resolveValue` now uses to walk the scope chain too. Every tagged template passed to `addCssTagCandidate` is marked as processed, whatever its tag. The README shows the supported interpolations in an example instead of a list, and formats css`` as code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`parseFile` now records the initializer of each `const` instead of evaluating it, and `resolveValue` evaluates an expression on demand: a literal, a signed number, or an identifier, whose `const` initializer is resolved in turn, in its own module for an imported binding. So a `const` bound to an import resolves too, and so does an exported alias in the modules importing it. The values are cached for the transform, keyed by initializer, unknown ones included. A binding is cached as unknown while it is resolved, so that a cycle stays unknown. Exports now map exported names to local names, so a binding exported under several names resolves under each of them. Re-exported imports are still not followed. The README example interpolates an imported class name, and no longer shows a `const` declared after the template that uses it, so that users do not rely on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: - src/index.ts: the single loop iterates the declarations map and resolves each interpolation with the lazy `resolveValue` Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
extractCssFromCodewent through the declarations twice, templates without interpolations first and the others second, then sorted the extracted CSS back into source order. The first pass dates from when extracting a template recorded its class name, so that the second pass could resolve it.parseFilerecords every class name now, so the two passes only reordered the extractions.This PR replaces them with a single loop over the declarations, which are already in source order, and removes what only served the two passes:
Declaration.hasInterpolationssourcePositionfieldaddProcessedDeclarationhelper and the separatereplacementsarray, as each extraction now records the range it replacesThe output does not change. The first pass did nothing besides collecting extractions, so the second pass's warnings and
this.load()calls still happen in the same order.Test plan
node --run test(locally and withCI=trueagainstdist/),lint,typecheck,format:check, andbuildpass, andsrc/index.tsstatement coverage stays at 99.6%🤖 Generated with Claude Code