Skip to content

Fix infinite forEachChild loop on some nodes in JS API #2833

Open
auvred wants to merge 2 commits intomicrosoft:mainfrom
auvred:cache-api-node-children-by-index
Open

Fix infinite forEachChild loop on some nodes in JS API #2833
auvred wants to merge 2 commits intomicrosoft:mainfrom
auvred:cache-api-node-children-by-index

Conversation

@auvred
Copy link
Contributor

@auvred auvred commented Feb 19, 2026

If we cache by text position rather than by node index, traversing namespace foo.bar {} ends up in an infinite loop.

Note: #2835 supersedes this PR. If the team plans to adopt the approach of #2835, this PR can be closed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an infinite forEachChild traversal loop in the JS API AST materialization by changing the child-cache keying strategy to use node indices (stable/unique) instead of text positions (can collide on some syntactic forms like namespace foo.bar {}).

Changes:

  • Cache RemoteNode / RemoteNodeList children by node index rather than by pos in _packages/api/src/node.ts.
  • Add a sync API regression test that traverses namespace foo.bar {} and fails fast if traversal loops.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
_packages/api/src/node.ts Switch _children cache key from text position to node index to prevent cache collisions and traversal cycles.
_packages/api/test/api.sync.test.ts Add regression test ensuring forEachChild traversal terminates correctly on dotted namespaces.

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

Comments