API: add getChildren and token getters to Node - #63893
API: add getChildren and token getters to Node#63893Andrew Branch (andrewbranch) merged 10 commits into
Conversation
Adds the remaining child/token getters to the API: getChildren, getChildCount, getChildAt, getFirstToken, getLastToken.
typescript-automation
Bot
added
the
For Uncommitted Bug
There was a problem hiding this comment.
Pull request overview
Adds child and token navigation getters to the native TypeScript Node API.
Changes:
- Implements child/token materialization and caching.
- Adds getters to local and remote nodes via generators.
- Adds comprehensive AST navigation tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tools/scripts/tsc/generate-ts-ast.ts |
Generates local node getters. |
tools/scripts/tsc/generate-encoder.ts |
Generates remote node getters. |
packages/typescript/test/sync/ast.test.ts |
Tests child/token navigation. |
packages/typescript/src/ast/factory.generated.ts |
Adds generated local getters. |
packages/typescript/src/ast/astnav.ts |
Implements navigation and caching. |
packages/typescript/src/ast/ast.ts |
Extends the public node interfaces. |
packages/typescript/src/api/node/node.generated.ts |
Adds generated remote getters. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Andrew Branch (andrewbranch)
left a comment
Andrew Branch (andrewbranch)
left a comment
There was a problem hiding this comment.
Thanks for your patience on this review!
| /** @internal */ | ||
| tokenCache?: Map<string, Node>; | ||
| /** @internal */ | ||
| childrenCache?: WeakMap<Node, readonly Node[]>; |
There was a problem hiding this comment.
This should just be a Map since it’s owned by the SourceFile that also owns the children.
| if (pos >= end) { | ||
| return; | ||
| } | ||
| const scanner = getScannerForSourceFile(sourceFile, pos); |
There was a problem hiding this comment.
This creates a new scanner for every run of tokens between nodes. Please pass a shared one down from whatever top-level functions eventually call into this.
| } | ||
|
|
||
| let pos = node.pos; | ||
| const consumed = new Set<Node>(); |
There was a problem hiding this comment.
I don't think this should be necessary—I think there was a bug in forEachChild that led you to add this, but it's been fixed since.
|
|
||
| let pos = node.pos; | ||
| const consumed = new Set<Node>(); | ||
| const processNode = (child: Node): undefined => { |
There was a problem hiding this comment.
You do need to skip NodeFlags.Reparsed subtrees, like findNextToken does.
github-project-automation
Bot
moved this from Not started
to Waiting on author
in PR Backlog
|
I made the changes as requested, the childrenCache is a plain Map now, getChildren uses one shared scanner per call, the consumed set is gone (you were right, it's no longer needed), and reparsed subtrees are skipped like in findNextToken. |
Andrew Branch (andrewbranch)
left a comment
Andrew Branch (andrewbranch)
left a comment
There was a problem hiding this comment.
Thank you! A lot of people are going to be excited to have these back 👍
github-project-automation
Bot
moved this from Waiting on author
to Needs merge
in PR Backlog
Head branch was pushed to by a user without write access
|
my bad, I forgot to run dprint fmt, haha |
Fixes #63892
Adds the remaining child/token getters to the native API's
Node:getChildren(),getChildCount(),getChildAt(),getFirstToken()andgetLastToken().Context
This is a re-submission of a contribution originally made to the typescript-go repo. I previously contributed the position/text getters in microsoft/typescript-go#4308, and this follow-up (completing microsoft/typescript-go#4216) was submitted as microsoft/typescript-go#4440. That PR was closed when development moved back to this repository, and the maintainers asked contributors to re-open their PRs here — so here it is.
Compared to the original PR, the only changes are path remappings to this repo's layout (
_packages/native-preview/→packages/typescript/,_scripts/→tools/scripts/tsc/) and a rebase onto currentmain. It also incorporates the review feedback from the original PR (reusingshouldSkipChildinstead of duplicating the JSDoc predicate).Validation
npm run generatereproduces the generated files (node.generated.ts,factory.generated.ts) with no extra diffnpm run buildinpackages/typescriptpassesnpm testinpackages/typescript: 612 tests pass, 0 fail (includes the newgetChildren/token getter tests intest/sync/ast.test.ts)