Skip to content
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

Maintain modifiers on Omit #31205

Merged
merged 4 commits into from May 10, 2019
Merged

Maintain modifiers on Omit #31205

merged 4 commits into from May 10, 2019

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?

@@ -1446,9 +1446,7 @@ type Extract<T, U> = T extends U ? T : never;
/**
* 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Omit from #31115 makes all properties required
5 participants