Skip to content

Rename type parameters when they are shadowed. - #61342

Merged
Jake Bailey (jakebailey) merged 1 commit into
microsoft:mainfrom
bloomberg:fix-shadowing-of-type-parameters
Mar 4, 2025
Merged

Rename type parameters when they are shadowed.#61342
Jake Bailey (jakebailey) merged 1 commit into
microsoft:mainfrom
bloomberg:fix-shadowing-of-type-parameters

Conversation

@dragomirtitian

Copy link
Copy Markdown
Contributor

Fixes#61334

Temporary fix. This solution will rename the type parameters. While this solution is valid it is a solution that is incompatible with what a different implementation of declaration emit could produce. This solution always renames the type parameters if they are shadowed even if this is not necessary. A correct solution would only rename them if there was an inference fallback that required access to one of the shadowed parameters.

exportclassA<T=any>{publicreadonlyShadowedButDoesNotRequireRenaming=<T>(): T=>{returnnullasany}}exportfunctionneedsRenameForShadowing<T>(){typeA=TreturnfunctionO<T>(t: A,t2: T){}}

The first example does not require renaming as the outer T is not used in the field type. This example could be emitted by an external tool as well. The second example does require renaming , but it can't be emitted by an external tool anyway so it's not a problem if we rename the type parameters.

I am working on a fix that does this, but it does seem important to have a fix that emits a valid declaration file.

@github-project-automationgithub-project-automationBot moved this to Not started in PR BacklogMar 3, 2025
@typescript-botTypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Mar 3, 2025
@jakebailey

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) pack this

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

CommandStatusResults
pack this✅ Started✅ Results

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
"devDependencies": {
"typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/164902/artifacts?artifactName=tgz&fileId=367DAA8289E2EC98AE8431B56A8D0D1624A7EC95EBF119690972DA4D79D4695B02&fileName=/typescript-5.9.0-insiders.20250304.tgz"
}
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.9.0-pr-61342-2".;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our contexts for shadowing/not in the full checker are approximate as well - we over-rename in many areas already. It's basically expected. What's not is rewriting any input nodes (ie, if we know we're keeping a node the user wrote, then we know that name is locked in and we have to issue an accessibility error when we try to reference it and can't reach it instead). Now, this does technically rewrite an input node, but only in areas the full checker path would have anyway, historically, so it's probably fine. It's not like it's a "semantic" rewrite, either - it's purely symbolic.

@github-project-automationgithub-project-automationBot moved this from Not started to Needs merge in PR BacklogMar 4, 2025
@jakebailey

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) cherry-pick this to release-5.8

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

CommandStatusResults
cherry-pick this to release-5.8✅ Started✅ Results

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey, Jake Bailey (@jakebailey)! I've created #61345 for you.

Daniel Rosenwasser (DanielRosenwasser) pushed a commit that referenced this pull request Mar 19, 2025
…e-5.8 (#61345)
Co-authored-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Jan 7, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Uncommitted BugPR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5.8.2 generates incorrect d.ts for generics in static method

7 participants

@dragomirtitian@jakebailey@typescript-bot@weswigham@sandersn@DanielRosenwasser@andrewbranch