This repository was archived by the owner on Oct 29, 2024. It is now read-only.
Fix Signature handling for unions and generics #390
Merged
+72
−35
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
chriskrycho
force-pushed
the
bugfix-signature
branch
4 times, most recently
from
f859dde to
499d266
Compare
chriskrycho
force-pushed
the
bugfix-signature
branch
from
499d266 to
7a9cf8d
Compare
When using a `keyof` type to check whether the type parameter for Glimmer Component is a `Signature` or the classic `Args`-only type, if we do not force TS to distribute over union types, it resolves the `keyof` check for union types with no shared members as `never`, and `never extends <anything>` is always true. This in turn meant that for all such unions, as well as for cases where users were providing generic types which could then be further extended in their own subclasses. Accordingly, introduce the standard technique TypeScript provides for opting into distributivity: conditional types are documented to support exactly this.
chriskrycho
force-pushed
the
bugfix-signature
branch
from
7a9cf8d to
ad429d5
Compare
chriskrycho
added a commit
to ember-modifier/ember-modifier
that referenced
this pull request
Apr 1, 2022
This is basically testing that the types supplied here do not have the problem fixed by glimmerjs/glimmer.js#390. Here, we do not have that issue, but the tests will also make sure we don't accidentally *introduce* that issue, either.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When using a
keyoftype to check whether the type parameter for Glimmer Component is aSignatureor the classicArgs-only type, if we do not force TS to distribute over union types, it resolves thekeyofcheck for union types with no shared members asnever, andnever extends <anything>is always true. This in turn meant that for all such unions, as well as for cases where users were providing generic types which could then be further extended in their own subclasses.Accordingly, introduce the standard technique TypeScript provides for opting into distributivity: conditional types are documented to support exactly this.
The first commit adds failing tests covering this; the second implements the fix.