Repository navigation
Closed
Labels
In DiscussionNot yet reached consensusNot yet reached consensusSuggestionAn idea for TypeScriptAn idea for TypeScript
Description
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
- ...
Reacted by Avi Vahl and Christoph Thiede
Metadata
Metadata
Assignees
Labels
In DiscussionNot yet reached consensusNot yet reached consensusSuggestionAn idea for TypeScriptAn idea for TypeScript
Activity
RyanCavanaugh commentedon Jan 12, 2021
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 commentedon Jan 26, 2021
Closing since the proposed change didn't result in performance improvements.