merge-deep: wrong types inferred when using partial interfaces

[salisbury-espinosa]

Bug description

the correct operation of this utility depends on the order of types!

Let's say there is interface Pokemon like this:

import type {MergeDeep} from 'type-fest';

interface Pokemon {
      name: string;
      type: string;
    }

valid :

    const testThroughAssert: Pokemon = {} as MergeDeep<
      Partial<Pokemon>,
      Pokemon
    >;

=> success

invalid:

    const testThroughAssert: Pokemon = {} as MergeDeep<
      Pokemon,
      Partial<Pokemon>
    >;

=> error TS2322: Type '{ name?: string | undefined; type?: string | undefined; }' is not assignable to type 'Pokemon'.

Repro

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBDAnmApnA3gWRVA5igERRTAF84AzKCEOAciVQFoKUBnGOgbgFgAofsAB2MHBQCGAYzQAFCAGsUICEIz84GzUPEgUALjgcow3Lz6bNjfYZjGhp9ZtL9+klRwTsYAFQAW1AFdcXwBBNjYcGAM5RWVVAF4McnE2OGw8QmIwAB5HCxilFQAaPM0ZcVhgcQAbbIK4gD5Shq4gA

[taiyakihitotsu]

I have submitted a PR.

However, I believe the latter test is inappropriate in light of the comments in merge-deep, so I have not fix it.
According to the comment, the order affects the result.

The correct test is probably the following in this case.

    const testThroughAssert: Partial<Pokemon> = {} as MergeDeep<
      Pokemon,
      Partial<Pokemon>
    >;

[salisbury-espinosa]

The correct test is probably the following in this case.

why?
there is a merger of Pokemon and Partial<Pokemon> happening there....
it will be correct

    const testThroughAssert: Pokemon = {} as MergeDeep<
      Pokemon,
      Partial<Pokemon>
    >;

because
MergeDeep<Pokemon, Partial<Pokemon>> must be equal MergeDeep<Partial<Pokemon>, Pokemon>
and both (MergeDeep<Pokemon, Partial<Pokemon>> and MergeDeep<Partial<Pokemon>, Pokemon>) must be equal Pokemon

[taiyakihitotsu]

@salisbury-espinosa

This is cited from my PR comment: #1319 (comment)

MergeDeep<Pokemon, Partial<Pokemon>> should be equivalent to Partial<Pokemon> according to this comment in merge-deep.d.ts:

type-fest/source/merge-deep.d.ts

Line 386 in c671aa2

- Properties that exist in both objects are merged if possible or replaced by the one of the source if not.

---

the source in this comment, it means the 2nd arg of MergeDeep.

export type MergeDeep<Destination, Source, Options extends MergeDeepOptions = {}> = MergeDeepWithDefaultOptions<
	SimplifyDeepExcludeArray<Destination>,
	SimplifyDeepExcludeArray<Source>,
	Options
>;

from

type-fest/source/merge-deep.d.ts

Line 489 in c671aa2

export type MergeDeep<Destination, Source, Options extends MergeDeepOptions = {}> = MergeDeepWithDefaultOptions<

The test cases fills this certainly.

// Test for readonly
type ReadonlyFoo = {
	readonly string: string;
	readonly number: number;
	boolean: boolean;
};
type ReadonlyBar = {
	number: number;
	readonly boolean: boolean;
};
declare const readonlyTest: MergeDeep<ReadonlyFoo, ReadonlyBar>;
expectType<{
	readonly string: string;
	number: number;
	readonly boolean: boolean;
}>(readonlyTest);

from

type-fest/test-d/merge-deep.ts

Line 190 in c671aa2

// Test for readonly

---

Based on this definition, optional properties can be treated the same way.
So, in the current case where Destination (1st argument) and Source (2nd argument) are swapped, the resulting type will be Source.

In this case, MergeDeep<Pokemon, Partial<Pokemon>> should return Partial<Pokemon>, MergeDeep<Partial<Pokemon>, Pokemon> should return Pokemon (as you posted.)

Note

My PR is based on this MY assumption.
I’m not sure if it exactly matches the author’s intended behavior.

Since there’s a bug in either scenario, I submitted this PR first.
If my reasoning is incorrect, I expect it will be pointed out.