API: add parameter to ts.forEachChild, that is passed through to the callback

[ajafff]

Suggestion

🔍 Search Terms

forEachChild closure

✅ Viability Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Add an optional parameter to ts.forEachChild that is passed through to the callback.

export function forEachChild<T, S>(node: Node, cbNode: (node: Node, state: S) => T | undefined, cbNodes?: (nodes: NodeArray<Node>, state: S) => T | undefined, state: S): T | undefined;
export function forEachChild<T>(node: Node, cbNode: (node: Node) => T | undefined, cbNodes?: (nodes: NodeArray<Node>) => T | undefined): T | undefined;

There's already a similar pattern in ts.forEach{Leading,Trailing}CommentRanges:

TypeScript/src/compiler/scanner.ts

Line 834 in 1ecf228

export function forEachLeadingCommentRange<T, U>(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state: T): U | undefined;

📃 Motivating Example

This would help API users and the compiler codebase itself to avoid closures in many cases when using forEachChild.

💻 Use Cases

Before (needs closure):

function analyzeNode(node: ts.Node) {
  const children: ts.Node[] = [];
  // the following closure prevents some opimizations of the containing function
  // e.g. 'children' is "context-allocated", i.e. on the heap instead of the stack,
  // a new instance of the closure is created every time the containing function is called,
  // ...
  ts.forEachChild(node, (child) => void children.push(child));
  // now do stuff with children
}

After (without closure):

function collectChildren(node: ts.Node, children: ts.Node[]) {
  children.push(node);
}

function analyzeNode(node: ts.Node) {
  const children: ts.Node[] = [];
  ts.forEachChild(node, collectChildren, undefined, children);
  // now do stuff with children
}

There are several functions in the compiler codebase, that could benefit from this:

• forEachReturnStatement
• forEachYieldExpression
• forEachChildRecursively
• getNodeAtPosition
• ...

[RyanCavanaugh]

Feel free to put up a PR for perf analysis purposes; I think if this had measurable gains we'd be inclined to take it

[ajafff]

Closing since the proposed change didn't result in performance improvements.