Can you weigh in on this, @rbuckton ?

@aozgaa We generally don't visit token nodes, as there is no context between a ? used to indicate an optional member vs. a ? used in a ConditionalExpression. Do you just need this in createParameter/updateParameter? If so, I'm ok with adding the parameter for those functions, but not in visitEachChild (e.g. do the same thing as https://github.com/Microsoft/TypeScript/pull/14642/files#diff-25d8529c0c0061fafa4aa188cab43277R248)

For @vladima's textChanges PR, we need to track the positions of all the nodes we will print. I was running into crashes if we didn't visit all the children and update them, including the tokens. Should we track/update this information in some other way?

The relevant usage of visitEachChild is here: https://github.com/Microsoft/TypeScript/blob/c687add579382a0910e985a36f88434312097dcc/src/services/textChanges.ts#L535

@aozgaa Visiting token nodes would add overhead to the transforms, though I'm not at the moment certain how significant that impact would be. You could add a special case to assignPositionsToNode to handle tokens in parameters like dotDotDotToken and questionToken to avoid this overhead.

Also, I'm curious why https://github.com/Microsoft/TypeScript/blob/c687add579382a0910e985a36f88434312097dcc/src/services/textChanges.ts#L539 uses a prototype assignment. Why not use Object.create(visited) instead?

Because I forgot about Object.create :) will change it.

It is not very convenient (though possible) to specifically recognize nodes that can contain tokens and then process tokens, I'd rather prefer to avoid special cases. What about adding optional tokenVisitor parameter to visitEachChild and if it is provided then it will be used to handle tokens, otherwise tokens from the original tree will be passed as it is done today. Standard transformations will not supply this argument so their behavior will not be affected.

@vladima That seems feasible.