-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Omit in 3.5 breaking Discriminated Unions #31501
Comments
|
All possible definitions of |
This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
A workaround for this is to use mapped types. E.g.,
instead of TS Playground example based on #39556 |
Found another workaround by using the fact that interface Dog { type: 'dog', name: string, barks: boolean }
interface Cat { type: 'cat', name: string, meows: boolean }
type Pet = Dog | Cat
type PetWithoutName = Omit<Pet, 'name'>
// Cannot because PetWithoutName is reduced to {type: 'dog' | 'cat'}
let unnamedPet:PetWithoutName = { type: 'dog', barks: true }
type OmitUnion<T, K extends keyof any> = T extends any ? Omit<T, K> : never
// Uses the fact that `T extends K` is applied to _each union member_ of T in-turn
type PetWithoutName2 = OmitUnion<Pet, 'name'>
let unnamedPet2:PetWithoutName2 = { type: 'dog', barks: true } So instead of ending up with the type It's kind of a hack - but I like that it enables us to have a generic re-usable |
@micdah yes, that is the way to get around it in https://github.com/unional/type-plus/blob/main/packages/type-plus/src/object/omit.ts |
Cool was not aware of those - but nice seeing multiple people coming up with the same workaround 👍 |
This is a dangerous API design. If someone designs an interface (as a non-discriminate union), dependents use it with Omit. There is a good chance that this will lead to software bugs, given the nature of the discriminate union is not propagated down to dependents (e.g. special handling of the discriminate union type for all cases). People aren't aware of these specifics, and keeping this in mind, figuring out all of its consequences is a hellscape of complexity. Problematic example code:
Before: export interface Item {
identifier: LearningPlanItemIdentifier;
type: 'topic';
item: TopicIdentifier;
} After: export type Item = ItemTopic | ItemUserGeneratedTopic;
export interface ItemTopic {
identifier: LearningPlanItemIdentifier;
type: 'topic';
item: TopicIdentifier;
}
export interface ItemUserGeneratedTopic {
identifier: LearningPlanItemIdentifier;
type: 'userGeneratedTopic';
item: UserGeneratedTopicIdentifier;
}
// Results in a type error
const full: Item = {
identifier: new LearningPlanItemIdentifier('foo'),
type: 'topic',
item: new UserGeneratedTopicIdentifier('sdf'),
};
// Does NOT result in a type error and in software bugs.
const partial: Omit<Item, 'identifier'> = {
type: 'topic',
item: new UserGeneratedTopicIdentifier('sdf'),
}; |
TypeScript Version: 3.5.0-rc, 3.5.0-dev.20190521
Search Terms: discriminated union omit, tagged union omit
Code
Expected behavior:
Omit
type should properly omit fields from objects but discriminated unions should still be discriminated unions. The expected behaviour is observed when usingOmit
from type-zoo package. The same approach was also proposed by the Microsoft member here: #28791 (comment).Actual behavior:
Built-in
Omit
type which is being introduced in TypeScript 3.5 "merges" discriminated unions. When omitting fieldc
fromthe resulting type should be equivalent to
but with Omit@3.5, we get
which is no longer a discriminated union.
According to Design Meeting Notes, 4/15/2019 (#30947), the resolution about making
Omit
stricter was based on "not worth breaking half the usages ofOmit
in the wild". When looking at "type Omit" search results, some of them also use the conditional type trick to distributeOmit
over discriminated union. Following the same "not breaking the usages in the wild" principle, it might be a better idea to change the built-inOmit
into the version withT extends any ? ... : never
. Especially since it does not break the regular use-cases.Playground Link: Link
The text was updated successfully, but these errors were encountered: