Generate names for type parameter declarations in inferred types#23902
Conversation
ghost
left a comment
There was a problem hiding this comment.
Should get another review though since I'm not very familiar with the emitter
| NoTruncation = 1 << 0, // Don't truncate result | ||
| WriteArrayAsGenericType = 1 << 1, // Write Array<T> instead T[] | ||
| // empty space | ||
| GenerateNamesForShadowedTypeParams = 1 << 2, // When a type parameter T is shadowing another T, generate a name for it so it can still be referenced |
There was a problem hiding this comment.
We don't wanna bother renaming the conflicts in quickinfo and signature help, at least in my opinion.
| function createFileLevelUniqueName(text: string): Identifier; | ||
| /** Create a unique name generated for a node. */ | ||
| function getGeneratedNameForNode(node: Node): Identifier; | ||
| function getOptimisticScopedGeneratedNameForName(node: Identifier): Identifier; |
| if (optimistic) { | ||
| if (checkFn(baseName)) { | ||
| generatedNames.set(baseName, true); | ||
| if (scoped) { |
There was a problem hiding this comment.
Anything that goes in generatedNames is already reserved throughout the entire generated output, so scoping these doesn't seem to make any sense. Why was this needed?
There was a problem hiding this comment.
scoped causes it to go into reservedNames instead of generatedNames; this allows, eg, T_1 to be reused in adjacent scopes.
| return name; | ||
| } | ||
|
|
||
| export function getOptimisticScopedGeneratedNameForName(node: Identifier): Identifier { |
There was a problem hiding this comment.
Do we need a special factory function for this? Couldn't we just add the extra flags to autoGenerateflags after calling one of the existing factory functions?
There was a problem hiding this comment.
Probably? But I'd also probably still write that into a function so it stays as an inline call as the use site.
There was a problem hiding this comment.
I'd rather we just change the signature of getGeneratedNameForNode to something like:
export function getGeneratedNameForNode(node: Node, flags?: GeneratedIdentifierFlags): Identifier
Also, the existing getGeneratedNameForNode actually defers reading the text of the node until emit. This is important so you could correctly handle a case where the node you are generating a name from is itself a generated name. What happens if you need to end up with <T>(x: T) => <T_1>(y: T_1) => T_1_1?
| return name; | ||
| } | ||
|
|
||
| export function getOptimisticScopedGeneratedNameForName(node: Identifier): Identifier { |
There was a problem hiding this comment.
I'd rather we just change the signature of getGeneratedNameForNode to something like:
export function getGeneratedNameForNode(node: Node, flags?: GeneratedIdentifierFlags): Identifier
Also, the existing getGeneratedNameForNode actually defers reading the text of the node until emit. This is important so you could correctly handle a case where the node you are generating a name from is itself a generated name. What happens if you need to end up with <T>(x: T) => <T_1>(y: T_1) => T_1_1?
|
@rbuckton Do you wanna update your status and give more comments? |
Fixes #16313