Make enum literal type identical to the union of its values #60417
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 test it |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
MichaelMitchell-at
changed the title
| @@ -21573,6 +21573,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| const t = isFreshLiteralType(type) ? (type as FreshableType).regularType : | |||
| isGenericTupleType(type) ? getNormalizedTupleType(type, writing) : | |||
| getObjectFlags(type) & ObjectFlags.Reference ? (type as TypeReference).node ? createTypeReference((type as TypeReference).target, getTypeArguments(type as TypeReference)) : getSingleBaseForNonAugmentingSubtype(type) || type : | |||
| type.flags & TypeFlags.EnumLiteral && type.flags & TypeFlags.Union ? getUnionType((type as UnionType).types) : | |||
There was a problem hiding this comment.
Not sure if I should cache the created type, e.g. type.resolvedReducedType || type.resolvedReducedType = getUnionType((type as UnionType).types).
Note that this must go above the next line, since getNormalizedUnionOrIntersectionType will just return back type, which will lead to the current iteration of the loop terminating.
|
@jakebailey would you mind kicking off tests again? |
jakebailey
reviewed
Nov 5, 2024
| @@ -91,54 +95,58 @@ enumAssignmentCompat3.ts(87,1): error TS2322: Type 'First.E' is not assignable t | |||
| abc = secondAbcd; // missing 'd' | |||
| ~~~ | |||
| !!! error TS2322: Type 'Abcd.E' is not assignable to type 'First.E'. | |||
| !!! error TS2322: Property 'd' is missing in type 'First.E'. | |||
| !!! error TS2322: Type 'E.a' is not assignable to type 'E'. | |||
There was a problem hiding this comment.
This error seems super weird; makes me think the PR isn't quite right.
There was a problem hiding this comment.
I think this is the expected, but not necessarily the most desirable, outcome of the implementation. Abcd.E is now treated as the union of its constituents, which is Abcd.a | Abcd.b | Abcd.c | Abcd.d. When you try to assign the union to First.E, we check that every type in the union is assignable to First.E. Since Abcd.a is the first type in the union that's not assignable to First.E, it's the one we report in the diagnostic.
There was a problem hiding this comment.
Actually looking at the other test cases I see that this is testing an existing behavior I wasn't aware of. If two enums are defined in different namespaces, have the same name, and have the same members, then the enums are mutually assignable. This is pretty strange to me but I'm sure there must be a reason if there are explicit tests for this behavior.
There was a problem hiding this comment.
My first guess is that it means you can use enums defined in different versions of the same library interchangeably.
|
@typescript-bot test it |
|
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
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.
Passes all existing test cases and Airtable codebase ("Identicality" checks are probably quite rare so unsurprising). Creating this in the hopes that we can use the test infra on this repo to run this against the corpus of other open source libraries.
Backlogmilestone (required)mainbranchhereby runtestslocallyFixes mmkal/expect-type#33
Fixes #46112