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

Use DeepPartial snippet instead of shallow Partial #234

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Starwort
Copy link

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

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
Copy link
Collaborator

macdja38 commented Dec 7, 2021

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
Copy link

RebeccaStevens commented Dec 7, 2021

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

@Starwort
Copy link
Author

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
Copy link

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.

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.

3 participants