Remove _this, _super, and _newTarget name conflict errors#22890
Conversation
|
ping @rbuckton |
| Unique = 3, // Unique name based on the 'text' property. | ||
| Node = 4, // Unique name based on the node in the 'original' property. | ||
| OptimisticUnique = 5, // Unique name based on the 'text' property, first instance won't use '_#' if there's no conflict | ||
| FileLevel = 6, // Optimistic unique name based on the 'text' property, first instance won't use '_#' if there's no conflict, using only the file identifiers list |
There was a problem hiding this comment.
Both FileLevel and OptimisticUnique seem more like flags than kinds, since they just augment the behavior of makeUniqueName in emitter.ts.
| writer(helper); | ||
| } | ||
| else { | ||
| writer(helper({ |
There was a problem hiding this comment.
Is there a reason you allocate an object just to pass a callback? Could this not just be the callback itself?
| return helpersEmitted; | ||
| } | ||
|
|
||
| function writeHelper(writer: (text: string) => void, helper: EmitHelper["text"], makeUniqueName: UniqueNameHandler) { |
There was a problem hiding this comment.
There's a lot of indirection going on here, and writeHelper only seems to be used once. It seems like it would be better to just keep this inline in emitHelpers rather than add more complexity through even more callbacks and indirection.
| } | ||
| else { | ||
| writer(helper({ | ||
| makeFileLevelUniqueName: name => makeUniqueName(name, isFileLevelUniqueName, /*optimistic*/ true), |
There was a problem hiding this comment.
This could be pulled out into a function to avoid allocating a new function object every time this is called.
| } | ||
|
|
||
| function emitHelpers(node: Node, writeLines: (text: string) => void) { | ||
| function emitHelpers(node: Node, writeLines: (text: string) => void, makeUniqueName: UniqueNameHandler) { |
There was a problem hiding this comment.
We might want to just consider moving this into the printer.
|
@rbuckton Done. |
| newLine?: NewLineKind; | ||
| omitTrailingSemicolon?: boolean; | ||
| noEmitHelpers?: boolean; | ||
| module?: CompilerOptions["module"]; |
There was a problem hiding this comment.
These should probably be /*@internal*/.
Instead of issuing an error, we just rename the variable we emit. This is a followup to my comment on #22886.