Repository navigation
Keep instantiated signature arguments out of parameter lists - #65497
Open
Jake Bailey (jakebailey) wants to merge 1 commit into
Open
Jake Bailey (jakebailey) wants to merge 1 commit into
Jake Bailey (jakebailey) wants to merge 1 commit into
Conversation
Signature type parameter lists contain declarations, not the types used to instantiate a call. Keeping concrete types there exposes invalid AST children through the public API. Store instantiated arguments in a lazy EmitContext side table used only for signature display instead of adding a pointer to every functionlike AST node. Copy this metadata through the normal clone and update hooks so regular and arrow-style signature printing retains its arguments. Cover declaration kinds, cloned and updated display nodes, and generic versus instantiated signatures through the sync and async public APIs.
Jake Bailey (jakebailey)
requested review from
Gabriela Araujo Britto (gabritto) and
Wesley Wigham (weswigham)
and
a balanced review from Copilot
November 14, 2026 04:13
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Signature serialization loses display arguments across the public API and type-parameter declarations in still-generic instantiations, while synthesized signature metadata accumulates for the checker’s lifetime.
3 open issues
What changed in this PR
Separates instantiated signature type arguments from type-parameter declarations in the compiler’s synthesized AST, using emit-context metadata to support hover rendering.
Changes:
- Stores signature display arguments separately from AST type-parameter lists.
- Updates the printer to use that metadata and preserves it when cloning or updating nodes.
- Adds checker and API regression tests for declaration structure and signature rendering.
| File | Description |
|---|---|
tsc/internal/printer/printer.go |
Prints signature arguments from emit-context metadata. |
tsc/internal/printer/emitcontext.go |
Stores display arguments and copies them to derived nodes. |
tsc/internal/checker/nodebuilderimpl.go |
Builds display arguments separately from type-parameter declarations. |
tsc/internal/checker/checker_test.go |
Tests AST structure, rendering, cloning, and updates. |
packages/typescript/test/sync/api.test.ts |
Adds synchronous API declaration-structure coverage. |
packages/typescript/test/async/api.test.ts |
Adds asynchronous API declaration-structure coverage. |
Review effort: Balanced
| if b.ctx.flags&nodebuilder.FlagsWriteTypeArgumentsOfSignature != 0 && signature.target != nil && signature.mapper != nil && len(signature.target.TypeParameters()) != 0 { | ||
| for _, parameter := range signature.target.TypeParameters() { | ||
| typeParameters = append(typeParameters, b.typeToTypeNode(b.ch.instantiateType(parameter, signature.mapper))) | ||
| typeArguments = append(typeArguments, b.typeToTypeNode(b.ch.instantiateType(parameter, signature.mapper))) |
| // } | ||
| // Instantiated arguments are display metadata, not type parameter declarations. | ||
| if len(typeArguments) != 0 { | ||
| b.e.SetSignatureTypeArguments(node, b.f.NewNodeList(typeArguments)) |
| if c.signatureTypeArguments == nil { | ||
| c.signatureTypeArguments = make(map[*ast.Node]*ast.TypeArgumentList) | ||
| } | ||
| c.signatureTypeArguments[node] = arguments |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hover sticks type parameter declarations where type parameter arguments should be. We then have to work around this in emit, the type is a lie, etc.
I think it would be wise to stop doing this and instead smuggle this information on the emit resolver instead, thus making the AST types correct so they could be checked in the future.