Skip to content

Fixed an issue with constraint of an infer type parameter not being fully instantiatable - #54205

Closed
Mateusz Burzyński (Andarist) wants to merge 2 commits into
microsoft:mainfrom
Andarist:fix/infer-extends-depending-on-type-variables
Closed

Fixed an issue with constraint of an infer type parameter not being fully instantiatable#54205
Mateusz Burzyński (Andarist) wants to merge 2 commits into
microsoft:mainfrom
Andarist:fix/infer-extends-depending-on-type-variables

Conversation

@Andarist

Copy link
Copy Markdown
Contributor

fixes#54197

@typescript-botTypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 10, 2023
function maybeCloneTypeParameter(p: TypeParameter) {
const constraint = getConstraintOfTypeParameter(p);
return constraint && (isGenericObjectType(constraint) || isGenericIndexType(constraint)) ? cloneTypeParameter(p) : p;
return constraint && couldContainTypeVariables(constraint) ? cloneTypeParameter(p) : p;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I keep thinking that perhaps there is a better check for this kind of thing - but I honestly don't know what it is. I already had a need for this a couple of times and I couldn't find anything else in the codebase that would already test this kind of stuff in a different way.

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.

This function was introduced back in #31455; I suspect that it was a targeted fix and personally I'm partial to what you're changing it to (it makes sense to me as an optimization; if we always called clone, all tests pass, it's just slower).

Wesley Wigham (@weswigham)

@typescript-botTypeScript Bot (typescript-bot) added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels May 10, 2023
@jakebailey

Copy link
Copy Markdown
Member

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the parallelized Definitely Typed test suite on this PR at 0519dfa. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based user code test suite on this PR at 0519dfa. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the abridged perf test suite on this PR at 0519dfa. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based top-repos suite on this PR at 0519dfa. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the tarball bundle task on this PR at 0519dfa. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2023

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the extended test suite on this PR at 0519dfa. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2023

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/154118/artifacts?artifactName=tgz&fileId=EBD30EA9E7439D2A06D6AD86127F8115C8CFC1EE7CA43284406A7EFDD55BF31102&fileName=/typescript-5.1.0-insiders.20230510.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.1.0-pr-54205-7".;

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..54205

Metricmain54205DeltaBestWorstp-value
Angular - node (v16.17.1, x64)
Memory used365,267k (± 0.01%)365,292k (± 0.01%)~365,243k365,353kp=0.298 n=6
Parse Time3.56s (± 0.52%)3.57s (± 0.70%)~3.54s3.61sp=0.366 n=6
Bind Time1.17s (± 0.47%)1.18s (± 1.03%)~1.17s1.20sp=0.855 n=6
Check Time9.63s (± 0.46%)9.61s (± 0.25%)~9.58s9.65sp=0.571 n=6
Emit Time7.89s (± 0.50%)7.88s (± 0.71%)~7.84s7.99sp=0.520 n=6
Total Time22.25s (± 0.25%)22.25s (± 0.27%)~22.17s22.34sp=1.000 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used193,306k (± 0.69%)192,714k (± 0.04%)~192,635k192,808kp=0.230 n=6
Parse Time1.60s (± 1.96%)1.61s (± 2.27%)~1.54s1.64sp=0.625 n=6
Bind Time0.83s (± 0.99%)0.83s (± 0.91%)~0.82s0.84sp=0.729 n=6
Check Time10.33s (± 0.52%)10.38s (± 0.71%)~10.28s10.44sp=0.295 n=6
Emit Time3.01s (± 0.56%)3.02s (± 0.95%)~3.00s3.07sp=0.683 n=6
Total Time15.77s (± 0.50%)15.84s (± 0.56%)~15.73s15.97sp=0.230 n=6
Monaco - node (v16.17.1, x64)
Memory used345,860k (± 0.00%)345,863k (± 0.00%)~345,848k345,876kp=0.810 n=6
Parse Time2.73s (± 0.71%)2.73s (± 0.19%)~2.72s2.73sp=1.000 n=6
Bind Time1.09s (± 0.47%)1.08s (± 0.77%)~1.08s1.10sp=0.533 n=6
Check Time7.86s (± 0.60%)7.89s (± 0.46%)~7.83s7.93sp=0.227 n=6
Emit Time4.46s (± 0.98%)4.46s (± 0.67%)~4.42s4.49sp=0.627 n=6
Total Time16.14s (± 0.55%)16.16s (± 0.39%)~16.09s16.24sp=0.688 n=6
TFS - node (v16.17.1, x64)
Memory used300,083k (± 0.01%)300,119k (± 0.01%)+36k (+ 0.01%)300,098k300,152kp=0.031 n=6
Parse Time2.17s (± 0.80%)2.18s (± 1.11%)~2.16s2.23sp=0.560 n=6
Bind Time1.23s (± 0.61%)1.25s (± 0.44%)+0.02s (+ 1.36%)1.24s1.25sp=0.008 n=6
Check Time7.28s (± 0.40%)7.32s (± 0.46%)+0.04s (+ 0.62%)7.27s7.37sp=0.042 n=6
Emit Time4.34s (± 0.72%)4.35s (± 0.66%)~4.31s4.39sp=1.000 n=6
Total Time15.02s (± 0.34%)15.10s (± 0.38%)~15.04s15.19sp=0.092 n=6
material-ui - node (v16.17.1, x64)
Memory used480,933k (± 0.01%)480,912k (± 0.00%)~480,872k480,935kp=0.471 n=6
Parse Time3.25s (± 0.36%)3.25s (± 0.19%)~3.24s3.26sp=1.000 n=6
Bind Time0.93s (± 0.59%)0.94s (± 0.43%)~0.93s0.94sp=0.282 n=6
Check Time17.88s (± 1.60%)17.75s (± 0.45%)~17.64s17.83sp=0.748 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time22.06s (± 1.35%)21.94s (± 0.35%)~21.83s22.02sp=0.748 n=6
xstate - node (v16.17.1, x64)
Memory used560,416k (± 0.02%)560,442k (± 0.02%)~560,335k560,604kp=0.748 n=6
Parse Time4.02s (± 0.34%)4.01s (± 0.27%)~4.00s4.03sp=0.066 n=6
Bind Time1.76s (± 0.66%)1.76s (± 0.46%)~1.75s1.77sp=1.000 n=6
Check Time3.06s (± 0.49%)3.08s (± 0.69%)~3.06s3.11sp=0.085 n=6
Emit Time0.09s (± 0.00%)0.09s (± 4.62%)~0.08s0.09sp=0.405 n=6
Total Time8.93s (± 0.21%)8.95s (± 0.20%)~8.93s8.97sp=0.126 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-148-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
BenchmarkNameIterations
Current542056
Baselinemain6

Developer Information:

Download Benchmark

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the top-repos suite comparing main and refs/pull/54205/merge:

Everything looks good!

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@Andarist

Copy link
Copy Markdown
ContributorAuthor

this was already fixed by #57362 so I opened a test-only PR here: #59760

@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

...infer X extends TypeParameter[] in conditional type inappropriate binding

5 participants

@Andarist@jakebailey@typescript-bot@sandersn@rbuckton