Uh oh!
There was an error while loading. Please reload this page.
Add nested call and new expressions as potential intra expression inference sites - #54183
Closed
Mateusz Burzyński (Andarist) wants to merge 15 commits into
Closed
Conversation
TypeScript Bot (typescript-bot)
commented
May 8, 2023
Contributor
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Nathan Shively-Sanders (sandersn)
requested a review
from Anders Hejlsberg (ahejlsberg)May 22, 2023 23:09
Nathan Shively-Sanders (sandersn)
requested a review
from Wesley Wigham (weswigham)May 22, 2023 23:09
Wesley Wigham (weswigham)
commented
May 24, 2023
Member
TypeScript Bot (@typescript-bot) test this |
Contributor
Heya Wesley Wigham (@weswigham), I've started to run the perf test suite on this PR at bcb0da5. You can monitor the build here. Update: The results are in! |
Contributor
Heya Wesley Wigham (@weswigham), I've started to run the diff-based top-repos suite on this PR at bcb0da5. You can monitor the build here. Update: The results are in! |
Contributor
Heya Wesley Wigham (@weswigham), I've started to run the parallelized Definitely Typed test suite on this PR at bcb0da5. You can monitor the build here. Update: The results are in! |
Contributor
Heya Wesley Wigham (@weswigham), I've started to run the extended test suite on this PR at bcb0da5. You can monitor the build here. |
Wesley Wigham (weswigham)
left a comment
Member
There was a problem hiding this comment.
This seems fine - iirc, needing to do a deeper traversal for context-sensitivity checking was something we've known for awhile, we've just been avoiding doing it in most of our other fixes, since we've been worried about the perf cost of a deeper tree traversal. Assuming perf comes back fine, only change I think I'd want is a deprecated isContextSensitive in the public API (which can just call the new function), so this isn't a hard API break.
TypeScript Bot (typescript-bot)
commented
May 24, 2023
Contributor
Heya Wesley Wigham (@weswigham), I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
TypeScript Bot (typescript-bot)
commented
May 24, 2023
Contributor
Wesley Wigham (@weswigham) Here are the results of running the top-repos suite comparing Everything looks good! |
TypeScript Bot (typescript-bot)
commented
May 24, 2023
Contributor
Hey Wesley Wigham (@weswigham), the results of running the DT tests are ready. |
…inference-with-resolved-calls # Conflicts: # src/compiler/checker.ts # tests/baselines/reference/intraExpressionInferences.errors.txt # tests/baselines/reference/intraExpressionInferences.js # tests/baselines/reference/intraExpressionInferences.symbols # tests/baselines/reference/intraExpressionInferences.types # tests/cases/conformance/types/typeRelationships/typeInference/intraExpressionInferences.ts
Mateusz Burzyński (Andarist)
commented
May 24, 2023
ContributorAuthor
is this relevant for a function that is annotated with |
TypeScript Bot (typescript-bot)
commented
May 24, 2023
Contributor
Wesley Wigham (@weswigham) Here they are:CompilerComparison Report - main..54183
System
Hosts
Scenarios
TSServerComparison Report - main..54183
System
Hosts
Scenarios
StartupComparison Report - main..54183
System
Hosts
Scenarios
Developer Information: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…inference-with-resolved-calls # Conflicts: # src/compiler/checker.ts
…inference-with-resolved-calls # Conflicts: # src/compiler/checker.ts # tests/baselines/reference/intraExpressionInferences.errors.txt # tests/baselines/reference/intraExpressionInferences.js # tests/baselines/reference/intraExpressionInferences.symbols # tests/baselines/reference/intraExpressionInferences.types # tests/cases/conformance/types/typeRelationships/typeInference/intraExpressionInferences.ts
Mateusz Burzyński (Andarist)
requested a review
from Wesley Wigham (weswigham)January 12, 2024 11:58
…inference-with-resolved-calls
Mateusz Burzyński (Andarist)force-pushed
the
fix/intra-expression-inference-with-resolved-calls
branch
from
January 12, 2024 12:08
68e3c49 to
bfdaa86CompareWesley Wigham (weswigham)
commented
Jan 16, 2024
Member
TypeScript Bot (@typescript-bot) perf test public |
Contributor
Heya Wesley Wigham (@weswigham), I've started to run the public perf test suite on this PR at bfdaa86. You can monitor the build here. Update: The results are in! |
TypeScript Bot (typescript-bot)
commented
Jan 16, 2024
Contributor
Wesley Wigham (@weswigham) Here they are:tscComparison Report - baseline..pr
System info unknown Hosts
Scenarios
Developer Information: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Wesley Wigham (weswigham)
commented
Jan 16, 2024
Member
TypeScript Bot (@typescript-bot) run dt |
Contributor
Heya Wesley Wigham (@weswigham), I've started to run the diff-based user code test suite on this PR at bfdaa86. You can monitor the build here. Update: The results are in! |
Mateusz Burzyński (Andarist)
commented
Mar 23, 2024
ContributorAuthor
…inference-with-resolved-calls
Mateusz Burzyński (Andarist)
commented
Jun 18, 2024
ContributorAuthor
#57909 was merged so the issue above is resolved now. Jake Bailey (@jakebailey) could you re-run top800 and other important tests here? Daniel Rosenwasser (@DanielRosenwasser)Wesley Wigham (@weswigham) could I ask for another review? :) |
Jake Bailey (jakebailey)
commented
Jun 18, 2024
Member
TypeScript Bot (@typescript-bot) perf test this |
Contributor
TypeScript Bot (typescript-bot)
commented
Jun 18, 2024
Contributor
Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready. Everything looks the same! |
TypeScript Bot (typescript-bot)
commented
Jun 18, 2024
Contributor
Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing Everything looks good! |
TypeScript Bot (typescript-bot)
commented
Jun 18, 2024
Contributor
Jake Bailey (@jakebailey) Here they are:tscComparison Report - baseline..pr
System info unknown Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown Hosts
Scenarios
Developer Information: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TypeScript Bot (typescript-bot)
commented
Jun 18, 2024
Contributor
Jake Bailey (@jakebailey) Here are the results of running the top 800 repos with tsc comparing Everything looks good! |
Dominik Dorfmeister 🔮 (TkDodo)
commented
May 12, 2025
Jake Bailey (@jakebailey) would it be possible to get a PR preview build with the changes from here? We think there might be an issue in react-query that’s related to this and I’d like to check if this PR fixes it: 🙏 |
…inference-with-resolved-calls
Mateusz Burzyński (Andarist)force-pushed
the
fix/intra-expression-inference-with-resolved-calls
branch
from
May 12, 2025 14:25
a9c578f to
32740a4CompareJake Bailey (jakebailey)
commented
May 19, 2025
Member
TypeScript Bot (@typescript-bot) pack this |
Contributor
Contributor
Hey Jake Bailey (@jakebailey), I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
Dominik Dorfmeister 🔮 (TkDodo)
commented
May 19, 2025
Thank you Jake Bailey (@jakebailey) ❤️ . Mateusz Burzyński (@Andarist)just confirmed that this PR fixes the issue, which is great news 🎉 |
…inference-with-resolved-calls
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR improves TypeScript's type inference for nested generic function calls by expanding the conditions under which expressions are considered as potential intra-expression inference sites. The change addresses a bug where nested generic functions were being skipped during the first pass of type inference, leading to incomplete type resolution.
Key changes:
- Renames
isContextSensitivetocontainsContextSensitivefor better clarity of purpose - Introduces
containsContextSensitiveOrCallOrNewExpressionto include call and new expressions as inference sites - Updates inference site detection to consider nested calls and constructors alongside context-sensitive functions
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/compiler/checker.ts | Core logic changes to expand inference site detection and rename context sensitivity checking function |
src/compiler/types.ts | Updates type interface to reflect function rename |
src/services/refactors/extractSymbol.ts | Updates function call to use renamed containsContextSensitive |
tests/cases/conformance/types/typeRelationships/typeInference/intraExpressionInferencesNestedGenericCall1.ts | Comprehensive test case for nested generic call inference with complex Fastify-style type provider pattern |
tests/cases/conformance/types/typeRelationships/typeInference/intraExpressionInferencesJsx.tsx | Additional JSX test case demonstrating improved inference |
tests/cases/conformance/types/typeRelationships/typeInference/intraExpressionInferences.ts | Extended test cases for both object and tuple parameter patterns |
| Various baseline files | Expected test outputs showing improved type inference results |
Comments suppressed due to low confidence (2)
…inference-with-resolved-calls
…inference-with-resolved-calls
…inference-with-resolved-calls
TypeScript Bot (typescript-bot)
commented
Mar 24, 2026
Contributor
With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies). Next steps for PRs:
|
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 freeto 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.
Sometimes nested generic functions are skipped during inference in the first pass:
https://github.dev/microsoft/TypeScript/blob/04d4580f4eedc036b014ef4329cffe9979da3af9/src/compiler/checker.ts#L33619-L33634
However, those calls are resolved from within
checkExpressionWithContextualTypeand by the time we have a chance to calladdIntraExpressionInferenceSitethey are already resolved. So it, imho, makes sense to actually use them as intra expression inference sites.fixes#54184
fixes#52114
fixes#53776