While I'd expect this to be strictly faster, can we get some perf numbers here? Additionally, please get sign-off from @mhegazy and @sheetalkamat as well.

Tested:

const benchmark = require("benchmark");
// If you test both typescripts in the same process, it seems that whichever runs second is slower.
// So must run this script twice, once with require("typescript") and once with below.
const ts = require("../../TypeScript/built/local/typescript.js");
const text = require("fs").readFileSync("jquery-3.3.1.js", "utf-8"); // Get the uncompressed version from http://jquery.com/download/
const file = ts.createSourceFile("a.js", text, ts.ScriptTarget.ESNext, /*setParentNodes*/ true);

const suite = new benchmark.Suite;
suite.add("countChildren", () => countChildren(file));
function countChildren(node) {
    let count = 1;
    for (const child of node.getChildren(file))
        count += countChildren(child);
    return count;
}
suite.on("complete", function() {
    this.forEach(({name, stats}) => console.log(`${name}: ${stats.mean * 1000}ms`));
});
suite.on("error", err => { throw err.target.error; });
suite.run();

On my machine it runs in about 10ms with either version. Don't see any difference. Wouldn't expect a difference since this PR just changed the location of the code and didn't change the actual operations needed to get the children.

@DanielRosenwasser @sheetalkamat @mhegazy Could someone review?

What is the impetus for this change?

This change doesn't really change the algorithm used to calculate a node's children. It just moves that algorithm out to a helper function that's not inside the context of NodeObject -- and it returns a value instead of assigning to a private property. The idea is that the code shouldn't be stateful when it's not necessary.

This change doesn't really change the algorithm used to calculate a node's children. It just moves that algorithm out to a helper function that's not inside the context of NodeObject -- and it returns a value instead of assigning to a private property. The idea is that the code shouldn't be stateful when it's not necessary.

the issue is that it is not clear that is is different :). for future PRs like this one consider splitting them into multiple commits, where the first is just moving the code around, and then subsequent commits are refactoring it. this way reviewers can see the incremental change tot he logic.

Made a simpler PR in #22938.

this one is fine. just one comment above.