Skip to content

Expose index and siblings on walk context#20109

Merged
RobinMalfait merged 3 commits into
mainfrom
feat/improve-walk
May 24, 2026
Merged

Expose index and siblings on walk context#20109
RobinMalfait merged 3 commits into
mainfrom
feat/improve-walk

Conversation

@RobinMalfait
Copy link
Copy Markdown
Member

This PR is a small improvement of the current walk implementation where we will expose the index and the siblings on the current context.

During a walk, we walk over objects that contain a nodes: [] field. The ctx.parent that already exists is a reference to the parent node, but ctx.siblings is a reference to the ctx.parent.nodes.

The ctx.index is the index of the current node we are walking in the siblings array. This way we can prevent the awkward ctx.parent?.nodes.indexOf(node) which is a bit silly because we already know the nodes we're walking and its index...

The ctx.parent can be null, but the ctx.siblings will never be null, this can be seen in a situation like this:

let ast: AstNode = [nodeA, nodeB]

walk(ast, (node, ctx) => {
  if (node === nodeA) {
    ctx.parent === null; // Because there is no parent
    ctx.siblings === ast; // Because that's the current list we're looping over

    // Before this PR, we would have to do something like:
    let siblings = ctx.parent?.nodes ?? ast
  }
})

In the above example, the ast is a separately variable, but if this was inlined, we would run into some issues:

walk([nodeA, nodeB], (node, ctx) => {
  if (node === nodeA) {
    ctx.parent === null; // Because there is no parent
    ctx.siblings === ast; // Because that's the current list we're looping over

    // At this point, there is no way to get to the `[nodeA, nodeB]` list
    // without moving it to a variable first.
  }
})

So, this PR doesn't change much, the additional information we track is already known information that is now exposed to the caller of the walk function.

In this PR we did update some usages and got rid of some awkward ctx.parent?.nodes ?? [] and ctx.parent.nodes.indexOf(…) usages.

Test plan

  • All tests still pass as expected

These are just references to other information we already have. But it
does mean that we can avoid things such as:

```
let idx = ctx.parent.nodes.indexOf(node)
```
@RobinMalfait RobinMalfait requested a review from a team as a code owner May 24, 2026 19:57
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 24, 2026

Confidence Score: 5/5

Safe to merge — all changes are mechanical refactors of pre-existing lookups using information that the walk already had internally.

The walk implementation correctly sets index and siblings before every callback invocation in both enter and exit phases, including after Replace mutations and when popping back to a parent frame after visiting children. The removed if (ctx.parent) guard had no real effect at root level (a root-level node at index 0 starts the backward scan at idx = -1, so the loop body never executes), and the ctx.parent?.nodes ?? []ctx.siblings substitution is semantically equivalent for non-root nodes and slightly more correct at root level. Tests are updated and provide direct assertions on the new fields.

No files require special attention.

Reviews (1): Last reviewed commit: "use `ctx.index` and `ctx.siblings`" | Re-trigger Greptile

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 00025031-63c4-4c50-adaf-ad7e4dde2129

📥 Commits

Reviewing files that changed from the base of the PR and between 982e920 and 68fefb0.

📒 Files selected for processing (6)
  • packages/@tailwindcss-upgrade/src/codemods/template/migrate-theme-to-var.ts
  • packages/tailwindcss/src/ast.ts
  • packages/tailwindcss/src/candidate.ts
  • packages/tailwindcss/src/canonicalize-candidates.ts
  • packages/tailwindcss/src/walk.test.ts
  • packages/tailwindcss/src/walk.ts

Walkthrough

This PR extends the AST walk context to expose sibling array traversal position. Two new fields are added to VisitContext: index (current position in sibling list) and siblings (the sibling array itself). The walk implementation is updated to initialize and maintain these fields during traversal. Multiple consumers throughout the codebase—arbitrary value formatting, CSS canonicalization, and theme migration—are refactored to use the new context fields instead of manually deriving node positions from ctx.parent.nodes, simplifying and centralizing position lookups.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: exposing index and siblings properties on the walk context, which is the core improvement across all modified files.
Description check ✅ Passed The description clearly relates to the changeset, explaining the motivation for exposing ctx.index and ctx.siblings, providing concrete code examples, and detailing how it eliminates awkward patterns like ctx.parent?.nodes ?? [].
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@RobinMalfait RobinMalfait merged commit 749c45e into main May 24, 2026
18 checks passed
@RobinMalfait RobinMalfait deleted the feat/improve-walk branch May 24, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant