TehShrike / deepmerge

A library for deep (recursive) merging of Javascript objects
MIT License
2.75k stars 216 forks source link

Use DeepPartial snippet instead of shallow Partial #234

Open Starwort opened 3 years ago

Starwort commented 3 years ago

In the spirit of the library, allow the typing support to use a deep partial instead of a shallow partial, removing type errors when reconstructing an object from a deep partial

macdja38 commented 2 years ago

I'm worried the typing still doesn't correctly represent what deep merge will return, especially given the more advanced custom merging options / etc.

This PR is definitely an improvement though, but it might stop existing code compiling.

RebeccaStevens commented 2 years ago

A more accurate typing would be to remove partial all together and TypeScript infer the type.

Starwort commented 2 years ago

A more accurate typing would be to remove partial all together and TypeScript infer the type.

I might be wrong, but I believe you are required to type function declarations; not declaring explicit types would error under --no-implicit-any and implicitly annotate as Any otherwise

RebeccaStevens commented 2 years ago

A more accurate typing would be to remove partial all together and TypeScript infer the type.

I might be wrong, but I believe you are required to type function declarations; not declaring explicit types would error under --no-implicit-any and implicitly annotate as Any otherwise

What I mean is that TypeScript can infer the generic. So for example:

// current definition
declare function deepmerge<T>(x: Partial<T>, y: Partial<T>, options?: deepmerge.Options): T;

// new definition where TS infers the generics
declare function deepmerge<T1, T2>(x: T1, y: T2>, options?: deepmerge.Options): T1 & T2;

When the function is called like deepmerge(foo, bar), if foo and bar are both typed as DeepPartial<SomeType>, then the output will also be typed as DeepPartial<SomeType>. No need for deepmerge to need to be aware of DeepPartial at all.

If foo is typed as { baz: string } and bar as { quux: number }; the output will be typed as { baz: string; quux: number }.

This approach does have limitations though. It can't handle the case of foo being { baz: string } and bar being { baz: number } as this will result in the output type being { baz: never } (seems string & number is never).

I've made a PR that much better types this library by breaking down how the merge actually happens. I've also made another PR to convert this project to TypeScript but it's been stale for awhile. I've released my own TypeScript version of this library in the meantime that has fully type support, even for custom merging. The type definitions for the library are actually longer than the actual implementation code.