Skip to content

Conversation

@DanielRosenwasser
Copy link
Member

Fixes #31190.

@DanielRosenwasser DanielRosenwasser changed the title Omit type modifiers Maintain modifiers on Omit May 1, 2019
@InExtremaRes
Copy link

It was nice to have a better error message with the name Omit in it... I suppose there is no way to have both 😅

BTW, why { [P in Exclude<keyof T, K>]: T[P] } doesn't preserve modifiers and Pick does?

/**
* Construct a type with the properties of T except for those in type K.
*/
type Omit<T, K extends keyof any> = {
Copy link
Contributor

@jwbay jwbay May 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔Why can't this be K extends keyof T to make the type homomorphic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that would make the type homomorphic, would it @ahejlsberg?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, discussed offline, but for reasons outlined below, the fact that it's not just a simple keyof T or K (where K extends keyof T) means that TypeScript can't trivially look at that and say "yup, we gotta keep the modifiers around on this one".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also do

type Omit<T, K extends keyof any> = T extends unknown ? Pick<T, Exclude<keyof T, K>> : never;

that is to say: making the builtin Omit distributive. (Which'd give it an alias symbol, too) A bunch of library authors end up swapping their internal Omit to this once they get into situations involving unions.

Distributive conditional like this have perf issues when nested repeatedly right now, tho, sooooo.... ehhhhh?

@DanielRosenwasser
Copy link
Member Author

BTW, why { [P in Exclude<keyof T, K>]: T[P] } doesn't preserve modifiers and Pick does?

What it comes down to is just the rules for when a mapped type decides it should maintain the modifiers, and right now it never does anything "smart" if the constraint (the thing after the in keyword is a conditional type.

It was nice to have a better error message with the name Omit in it...

Yeah. I agree. ☹

@DanielRosenwasser DanielRosenwasser merged commit 39e9a2b into master May 10, 2019
@DanielRosenwasser DanielRosenwasser deleted the omitTypeModifiers branch May 10, 2019 17:03
@millsp
Copy link
Contributor

millsp commented May 21, 2019

@DanielRosenwasser is it planned to get this smart "in" in the near future ?

@DanielRosenwasser
Copy link
Member Author

I'm not sure what you're referring to - the functionality here was implemented. If you have a specific suggestion for new/different functionality in mapped types it's probably best to create a separate issue.

@DanielRosenwasser
Copy link
Member Author

I'm not sure what you're referring to - the changes here were merged and will be in TypeScript 3.5. If you have a specific suggestion for new/different functionality in mapped types it's probably best to create a separate issue.

@millsp
Copy link
Contributor

millsp commented May 21, 2019

Sorry for the confusion... So you reverted the Omit back to what is was because it caused the modifiers to be discarded (optional, readonly). And you explained that this is happening because the in keyword only maintains the modifiers with keyof (but not conditional types).

So my question is: will in also preserve modifiers when used with conditional types in the future?

@DanielRosenwasser
Copy link
Member Author

I think it is possible, but that change is not something I can personally dedicate a ton of time towards, and there are other possible changes that are more in-demand at the moment.

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Omit from #31115 makes all properties required

6 participants