omit typing?

[aradalvand]

Hi there, thanks for this immensely useful library.

The current signature of the omit function is as follows:

function omit<Obj extends object, Key extends string>(obj: Obj, remove: Key[]): Omit<Obj, Key>;

This doesn't make sense, it means that you get no type-safety whatsoever when specifying the property keys, they would effectively be magic strings. Why not:

function omit<T extends object>(obj: T, remove: (keyof T)[]);

This, on the other hand, does provide type-safety and all the goodness that comes with it (e.g. code completion, etc.)

[Masa-Shin]

Thanks, I will fix this.

[Masa-Shin]

Now I remember why the type definition is like that. In the examples of the package you can find omit(obj, ['a', 'b', 'd']); // {c: 9}, where d is not a key of obj. So it accepts any strings as the second argument (like typescript's Omit<Obj, Key> does). I don't think there's a way to make code completion work in such case.

[aradalvand]

@Masa-Shin Okay but then why isn't pick like this as well? pick and omit at least have to be consistent, don't they?

pick's signature looks like this:

function pick<T, U extends keyof T>(obj: T, select: U[]): Pick<T, U>;

And there's also an example like the one you mentioned for pick here:

pick(obj, ['a', 'b', 'd']); // {a: 3, b: 5}

This example actually yields an error:

So at the very least, these two functions need to be consistent in their typing.

But I would argue omit should become like pick and not the other way around, because omit now essentially provides zero type-safety and that's not good, if somebody wanted to add a property key to the second argument that doesn't exist in the type, they could simply opt for type assertions:

This is actually how you typically handle edge cases like this in TypeScript. I would argue the library should just provide the most expected typing.

[Masa-Shin]

I see your point. I made omit's typing reflect the actual behavior of the function for the convenience of those who already have used it without TS (as they might have passed a list of arbitrary strings to it), but I realized that people using TS may be looking for safety over convenience so now I rather agree with your suggestion.

[Masa-Shin]

I think it is OK to make a PR for the issue since that for the pick has already been merged. If it is OK with you, I will do it today (for there are some other things I would like to fix about the function).

[aradalvand]

but I realized that people using TS may be looking for safety over convenience

Yes, exactly!

I think it is OK to make a PR for the issue since that for the pick has already been merged. If it is OK with you, I will do it today. (for there are some other things I would like to fix about the function)

Yeah I think that makes sense, that would be great, thank you so much! :)

[angus-c]

Thanks @aradalvand and @Masa-Shin! PR landed