Use key remapping from Typescript 4.1 for PickProperties and OmitProperties #395
Conversation
|
@DanielBertocci Hey again! 👋 I really appreciate you took your time to improve some of the types. As the current But before merging the changes, let me quickly analyse what's the performance gain (the diff between the "before" and the "after" in terms of instantiations) because I think it's a significant improvement (FYI I'm doing benchmarks in this PR - #393) |
| import { PickKeysByValue } from "../pick-keys-by-value"; | ||
|
|
||
| export type OmitProperties<Type, Value> = Omit<Type, PickKeysByValue<Type, Value>>; | ||
| export type OmitProperties<Type, Value> = { [Key in keyof Type as Type[Key] extends Value ? never : Key]: Type[Key] }; |
There was a problem hiding this comment.
Amazing work! It's very performant right now
| @@ -1,3 +1,3 @@ | |||
| import { PickProperties } from "../pick-properties"; | |||
|
|
|||
| export type PickKeys<Type, Value> = Exclude<keyof PickProperties<Type, Value>, undefined>; | |||
| export type PickKeys<Type, Value> = keyof PickProperties<Type, Value>; | |||
There was a problem hiding this comment.
Ha, it's amazing that key remapping fixed the problem with undefined
There was a problem hiding this comment.
@DanielBertocci LGTM! I don't see any regressions as all tests are passed. I will update the instantiations and create changelog in a separate PR so I'm happy for this PR to be merged now
|
Great! Just as remark that this PR introduces a breaking change on the edge cases:
|
|
@DanielBertocci I am ready to accept the risk that this is a very unrealistic edge case therefore I will leave it as a "patch" anyway. The majority of the cases should apply this type to the object. |
PR Checklist
Overview
I took advantage of key remapping feature introduced starting from Typescript 4.1.
This allows to reduce the number of iterations when these types are used and to remove the utility PickKeysByValue.
In tests, the "wtf" cases continue to be a bit "wtf" but they are closer to be keys, at least.
I wanted to use this syntax for other cases, but it changes the behavior a bit more and, despite I think it would be reasonable the new behavior, it requires more discussion.