Implement variable scoping and shadowing support#14
Draft
Conversation
The plugin was using a flat Map for local identifiers, causing inner scope shadows (functions, blocks, arrows) to overwrite module-level values. This led to incorrect CSS interpolation resolution. Replace the flat map with a scope chain that tracks identifiers per BlockStatement. Each CSS declaration captures its scope, and resolution walks up the chain to find the nearest binding. Add comprehensive scoping tests covering function shadows, nested functions (3 levels), arrow functions, block scopes, import shadowing, CSS class interpolation across scopes, var declarations, multiple simultaneous shadows, and sequential blocks. https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
Scope handling has been implemented via BlockStatement-based scope chain tracking. https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Instead of importing from @oxc-project/types (not a direct dependency), extract the type from rolldown's VisitorObject type. https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
…roject/types The package is hoisted by npm as a transitive dependency of rolldown. https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
Track scopes for ForStatement, ForInStatement, ForOfStatement, SwitchStatement, CatchClause, FunctionDeclaration, FunctionExpression, and ArrowFunctionExpression in addition to BlockStatement. This prevents incorrect resolution of shadowed variables in these contexts (e.g. for-loop variables shadowing outer const declarations). https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
Major scope tracking improvements: - Record function/arrow parameters in scope as unknown values so they correctly shadow outer variables (prevents incorrect resolution of css`` interpolations that reference parameters) - Record catch clause parameters in scope for proper shadowing - Record all VariableDeclarators regardless of init type (let x;, const x = fn()) so they shadow outer variables - Merge parent scope with child BlockStatement to avoid redundant double scoping (function body, for body, catch body) - Add StaticBlock scope to prevent class static block variables from leaking to module scope - Change Scope.identifiers to Map<string, string | undefined> where undefined means "declared but value unknown at build time" https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
… Set Use a Set<object> of body nodes instead of a boolean flag + boolean[] stack. Parent nodes (functions, for-statements, catch) add their body BlockStatement to the set, and the BlockStatement handler checks it to skip redundant scope creation. https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
- Use Set<BlockStatement> instead of Set<object> with proper type import - Check node.body.type === 'BlockStatement' consistently in all handlers (functions, arrow functions) instead of mixing !== null and !expression - Refactor VariableDeclarator to switch pattern with early returns, falling through to shadow-only recording as the default https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
In strict mode (ESM), function and class declaration names create bindings in the containing scope. Record these names so they properly shadow outer variables: - FunctionDeclaration: name recorded in parent scope (before pushScope) - FunctionExpression: name recorded in function scope (after pushScope), since it's only visible inside the function body for recursion - ClassDeclaration: name recorded in parent scope https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
Add recursive recordBindingPattern function that extracts all binding
identifiers from ObjectPattern, ArrayPattern, AssignmentPattern, and
RestElement nodes. This ensures destructuring declarations like
const [color] = arr, const { color } = obj, and destructured function
parameters properly shadow outer variables.
Also use recordBindingPattern for catch clause params (handling
destructured catch like catch ({ message })).
https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
…scoping test Much easier to read and understand the expected output at a glance, and less brittle than matching individual patterns with regexps. https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB
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
2 participants
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
This PR adds proper variable scoping and shadowing support to the CSS-in-JS plugin. Previously, the plugin used a flat map of identifiers, which didn't account for JavaScript's lexical scoping rules. Now it correctly resolves variables based on their scope chain, allowing inner scopes to shadow outer scope variables.
Key Changes
Scope Chain Implementation: Introduced a
Scopeinterface that maintains a chain of scopes with parent references, enabling proper lexical scope resolutionVariable Resolution: Updated
resolveValue()to walk up the scope chain instead of using a flat identifier mapScope Tracking: Added visitor hooks for
BlockStatemententry/exit to manage the current scope during AST traversalDeclaration Scope Association: Each CSS declaration now stores a reference to its scope, enabling correct variable resolution at code generation time
Export Handling: Modified export recording to only apply at module-level (root scope), preventing function-local declarations from being incorrectly exported
Implementation Details
currentScopevariablescoping.input.ts) covers 10 different scoping scenarios including nested functions, arrow functions, block scopes, and variable shadowingTesting
Added extensive test coverage with a new fixture file containing 10 test cases covering:
https://claude.ai/code/session_01Dbp3WX1HM1choWg55p9VhB