Filter out non-array types when contextually typing array literal elements #52589
Conversation
This file contains 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
typescript-bot
added
the
For Uncommitted Bug
Andarist
commented
Feb 3, 2023
|
@typescript-bot test this |
|
Heya @RyanCavanaugh, I've started to run the diff-based top-repos suite on this PR at 8ebc95f. You can monitor the build here. Update: The results are in! |
|
Heya @RyanCavanaugh, I've started to run the perf test suite on this PR at 8ebc95f. You can monitor the build here. Update: The results are in! |
|
Heya @RyanCavanaugh, I've started to run the diff-based user code test suite on this PR at 8ebc95f. You can monitor the build here. Update: The results are in! |
|
Heya @RyanCavanaugh, I've started to run the diff-based top-repos suite (tsserver) on this PR at 8ebc95f. You can monitor the build here. Update: The results are in! |
|
Heya @RyanCavanaugh, I've started to run the parallelized Definitely Typed test suite on this PR at 8ebc95f. You can monitor the build here. |
|
Heya @RyanCavanaugh, I've started to run the diff-based user code test suite (tsserver) on this PR at 8ebc95f. You can monitor the build here. Update: The results are in! |
|
Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at 8ebc95f. You can monitor the build here. |
Andarist
changed the title
|
@RyanCavanaugh Here are the results of running the user test suite comparing Everything looks good! |
|
@RyanCavanaugh Here are the results of running the user test suite comparing Something interesting changed - please have a look. Details |
|
Heya @RyanCavanaugh, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
|
When it comes to the new error reported for webpack here... I see that |
|
@RyanCavanaugh Here are the results of running the top-repos suite comparing Everything looks good! |
|
@RyanCavanaugh Here they are:
CompilerComparison Report - main..52589
System
Hosts
Scenarios
TSServerComparison Report - main..52589
System
Hosts
Scenarios
StartupComparison Report - main..52589
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@RyanCavanaugh Here are the results of running the top-repos suite comparing Everything looks good! |
The code is here: https://github.com/microsoft/typescript-error-deltas And the test (https://github.com/microsoft/typescript-error-deltas/blob/main/userTests/webpack/test.json) implies that it's just cloning and running tsc. |
|
Thank you for the pointers! It's a little bit weird that the bot's message mentioned |
|
I glossed over the fact that the error-deltas script has some logic to find tsconfigs and test them; that might be what's happening here. |
|
I've added this to https://github.com/microsoft/TypeScript/wiki/All-The-Bots |
|
TIL |
|
I confirmed that this webpack-related failure is caused by some weird intersection of this PR and this regression that was introduced in #51865 . The PR that is fixing this regression (this one) is fixing the webpack issue (I tested a build with this PR + @ahejlsberg's fix in it). I'll try to distill a minimal repro from webpack to include it as a test somewhere but I don't think it's going to be that crucial for this PR here. Once @ahejlsberg's PR lands - I will just pull in |
|
@RyanCavanaugh I pulled in the contextual type caching fix here (by merging |
|
@typescript-bot test this |
jakebailey
reviewed
Mar 8, 2023
| @@ -23075,6 +23065,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| return isArrayLikeType(type) || isTupleLikeType(type); | |||
| } | |||
|
|
|||
| function isAssignableToAvailableAnyIterable(type: Type): boolean { | |||
There was a problem hiding this comment.
Maybe I'm brainfarting, but what does "Available" mean in this context?
There was a problem hiding this comment.
When we are in es5 context the "available any iterable" becomes anyReadonlyArray. I know that the name isn't the best but I wanted to indicate that it might not exactly be testing against anyIterable (cause that might not be available). I'm open to suggestions for a better name here
|
@typescript-bot test this |
|
Heya @jakebailey, I'm starting to run the diff-based top-repos suite on this PR at 85c03c5. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya @jakebailey, I'm starting to run the diff-based user code test suite on this PR at 85c03c5. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya @jakebailey, I'm starting to run the parallelized Definitely Typed test suite on this PR at 85c03c5. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya @jakebailey, I'm starting to run the abridged perf test suite on this PR at 85c03c5. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Heya @jakebailey, I'm starting to run the extended test suite on this PR at 85c03c5. Hold tight - I'll update this comment with the log link once the build has been queued. |
|
Apparently the bot choked and I didn't notice. Let's try again: @typescript-bot test this |
|
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 85c03c5. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 85c03c5. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 85c03c5. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 85c03c5. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the extended test suite on this PR at 85c03c5. You can monitor the build here. |
|
@jakebailey Here are the results of running the user test suite comparing Everything looks good! |
|
@jakebailey Here they are:Comparison Report - main..52589
System
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hey @jakebailey, the results of running the DT tests are ready. Branch only errors:Package: dom-mediacapture-transform Package: dom-screen-wake-lock Package: web-animations-js Package: webappsec-credential-management Package: dom-webcodecs Package: w3c-css-typed-object-model-level-1 Package: use-color-scheme Package: dom-mediacapture-record Package: ramda |
|
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
jakebailey
approved these changes
Mar 10, 2023
Andarist
deleted the
fix/prefer-array-members-for-contextual-type-of-arr-el
branch
|
This PR breaks the following code: function f<T extends { "0": (p1: number) => number }>(p: T): T {
return p;
}
var v = f([x => x]); // Implicit any error that wasn't there beforeI'm surprised this wasn't caught by our CI tests since it is an existing test in Also, I'm not sure I agree with the issue (#52588) that this PR fixes. Seems to me entirely reasonable that the string index signature contributes to the contextual type for the array literal element. |
|
I think I understand why we didn't see a test failure. It's because we run our tests with a |
|
I'll send a revert. |
jakebailey
added a commit
to jakebailey/TypeScript
that referenced
this pull request
Mar 16, 2023
…eral elements (microsoft#52589)" This reverts commit e775383.
Merged
Yeah, the default lib is |
jakebailey
added a commit
to jakebailey/TypeScript
that referenced
this pull request
Mar 16, 2023
…eral elements (microsoft#52589)" This reverts commit e775383.
jakebailey
added a commit
that referenced
this pull request
Mar 16, 2023
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.
fixes #52588