Add `MergeDeep` type

[skarab42]

Merge two types recursively into a new type.

Properties set to undefined value are skipped when strict option is set to true (default). Array and plain object properties are merged recursively.

type Foo = {foo: string; bar: {id: string; label: string}};
type Bar = {foo: number; bar: {id: number; nop: undefined}};

type FooBar = MergeDeep<Foo, Bar>;

// {
// 	foo: number;
// 	bar: {
// 		id: number;
// 		label: string;
// 	}
// }

type FooBarLazy = MergeDeep<Foo, Bar, {strict:false}>;

// {
// 	foo: number;
// 	bar: {
// 		id: number;
// 		label: string;
//		nop: undefined;
// 	}
// }

@ilchenkoArtem @sindresorhus SetProp is comming soooon!

[sindresorhus]

Make sure you look at our existing "deep" types and that this new types handles all the cases handled by them.

For example, https://github.com/sindresorhus/type-fest/blob/2f418dbbb6182c53cdac26dc3421f7e8806c789b/source/readonly-deep.d.ts#L80

[skarab42]

Make sure you look at our existing "deep" types and that this new types handles all the cases handled by them.

For example,

https://github.com/sindresorhus/type-fest/blob/2f418dbbb6182c53cdac26dc3421f7e8806c789b/source/readonly-deep.d.ts#L80

Sorry, I'm not sure what I'm supposed to test. Can you give me a concrete example?

[sindresorhus]

Sorry, I'm not sure what I'm supposed to test. Can you give me a concrete example?

I gave you a concrete example: HasMultipleCallSignatures. Relevant commit and tests: https://github.com/sindresorhus/type-fest/commit/db5402803d06b3cda0565f4a43fc782929364efc

[sindresorhus]

Make sure you look at our existing "deep" types and that this new types handles all the cases handled by them.

Some other examples:

• Should it recurse into arrays?
• Should it recurse into Sets/Maps?

[skarab42]

• Should it recurse into arrays?
• Should it recurse into Sets/Maps?

At first I just wanted to contribute to a project I 💜 and improve my skills in the process by adding the SetProp type. While prototyping, I realized that I would need a Replace type to normalize the dotted paths. Then in several discussions I saw the need for the DeepMerge type, which would also simplify the creation of the SetProp type. Now, what I would love is to be able to type lodash.merge, but this is not necessarily what you need in type-fest.

In any case I think I have to start all over again and define before going any further the needs for this type.

If we want to go in the direction of lodash here is a non-exhaustive list cases I have been able to define:

// If the first type (the destination) is [number, string, boolean] return the destination.
_.merge(42, 24); // 42 
_.merge("42", "24"); // "42" 
_.merge(true, false); // true 

// [undefined, null, symbol, Set, Map] are always treated as empty objects regardless of location.
_.merge(undefined, null); // {} 
_.merge(null, undefined); // {} 
_.merge(Symbol(42), Symbol(24)); // {}
_.merge(new Set([42]), new Set([24])); // {} 
_.merge(new Map([["life", 42]]), new Map([["life", 42]])); // {}  

// If the first type is [Class, Function] return undefined.
_.merge(() => 42, {life:42}); // undefined 
_.merge(Math.floor, {life:42}); // undefined 
_.merge(Buffer, {life:42}); // undefined 

// If the first type is an object and the second is an array, the array is treated as an indexed object.
_.merge({"id":42}, [42]); // {"0":42,"id":42} 
_.merge(null, [42]); // {"0":42} // <- remeber null is treated as empty object

// If the first type is an array and the second is an object, the object is treated as an numerical indexed array stripping any string indexed properties.
_.merge([42], {"0":24,"id":42}); // [24] 
_.merge([42], null); // [42] 

// Instance of class is treated as an plain object.
class Foo {
  constructor() {
    this.id = "foo";
    this.foo = true;
  }
}

class Bar {
  constructor() {
    this.id = "bar";
    this.bar = true;
  }
}

_.merge(new Foo(), new Bar()); // {"id":"bar","foo":true,"bar":true}

[sindresorhus]

Now, what I would love is to be able to type lodash.merge, but this is not necessarily what you need in type-fest.

I don't really see the usefulness of matching lodash.merge. It has its own types anyway.

I would prefer to focus on keeping it simple and just make it a deep version of Merge.

We still need to answer whether it should recurse into arrays/maps/sets. My opinion is that it should not recurse into arrays by default, but have an option to opt-into it. It should not recurse into maps/sets. Thoughts?

Relevant: https://github.com/sindresorhus/type-fest/pull/400

[skarab42]

I don't really see the usefulness of matching lodash.merge. It has its own types anyway.

I would prefer to focus on keeping it simple and just make it a deep version of Merge.

We still need to answer whether it should recurse into arrays/maps/sets. My opinion is that it should not recurse into arrays by default, but have an option to opt-into it. It should not recurse into maps/sets. Thoughts?

I agree with all ;)

[sindresorhus]

Relevant: https://github.com/sindresorhus/type-fest/pull/400

[sindresorhus]

Also:

Please help review the other open pull requests. - https://github.com/sindresorhus/type-fest/blob/main/.github/contributing.md#submitting-a-new-type

🙏

[sindresorhus]

Maybe you have an opinion on https://github.com/sindresorhus/type-fest/issues/402 (write it in the issue) No worries if not.

[skarab42]

I'm setting up the new signature for DeepMerge and I need you all to define two slightly confusing cases.

// The value "42" represents any type that is neither an record nor an array.

MergeDeep<{}, {}>; // {}
MergeDeep<[], []>; // never

MergeDeep<{}, []>; // never
MergeDeep<[], {}>; // never

MergeDeep<42, {}>; // never
MergeDeep<42, []>; // never

MergeDeep<{}, 42>; // never
MergeDeep<[], 42>; // never

MergeDeep<{}, {}, {recurseIntoArrays: true}>; // {}
MergeDeep<[], [], {recurseIntoArrays: true}>; // []

MergeDeep<{}, [], {recurseIntoArrays: true}>; // ???
MergeDeep<[], {}, {recurseIntoArrays: true}>; // ???

MergeDeep<42, {}, {recurseIntoArrays: true}>; // never
MergeDeep<42, [], {recurseIntoArrays: true}>; // never

MergeDeep<{}, 42, {recurseIntoArrays: true}>; // never
MergeDeep<[], 42, {recurseIntoArrays: true}>; // never

I think the two cases should be treated differently.

For the first case, If the first type (the destination) is an object then the second type is treated as an object.

MergeDeep<{life: 42, "1": 42}, [true, false], {recurseIntoArrays: true}>;
// {life: 42, "0": true, "1": false}

The second case is a little more ambiguous. I think you have to treat the second type as an array, but how do you handle indexes that are not defined?

MergeDeep<[true, false], {life: 42, "1": 42}, {recurseIntoArrays: true}>;
// [true, 42] or [null, 42]

I lean more towards the second answer [null, 42] because if you break it down you see this:

// {life: 42, "1": 42} -> {"0": null, "1": 42} -> [null, 42]
MergeDeep<[true, false], [null, 42], {recurseIntoArrays: true}>;
// [null, 42]

[skarab42]

And how should "recurseIntoArrays" influence the merge of properties that are arrays?

MergeDeep<{items: [1, 2, 3, 4, 5, 6]}, {items: ['a', 'b', 'c']}>;
// 1. {items: ['a', 'b', 'c']} <---- my choice
// 2. {items: ['a', 'b', 'c', 4, 5, 6]}
// 3. {items: never}
// 4. never

MergeDeep<{items: [1, 2, 3, 4, 5, 6]}, {items: ['a', 'b', 'c']}, {recurseIntoArrays: true}>;
// 1. {items: ['a', 'b', 'c']}
// 2. {items: ['a', 'b', 'c', 4, 5, 6]} <---- my choice

~~For me "recurseIntoArrays" should only influence the first call and should traverse and merge all child arrays regardless of whether "recurseIntoArrays" is true or false. This means that in all cases, proposal n°2 is applied.~~

[sindresorhus]

MergeDeep<{}, [], {recurseIntoArrays: true}>; // ??? MergeDeep<[], {}, {recurseIntoArrays: true}>; // ???

Both should be never

For the first case, If the first type (the destination) is an object then the second type is treated as an object.

Arrays are techically objects, but it's better to treat them as not in general. I don't think anyone is gonna do this in practice and it's more likely it's a mistake, which we should catch.

// 1. {items: ['a', 'b', 'c']} <---- my choice

:+1:

// 2. {items: ['a', 'b', 'c', 4, 5, 6]} <---- my choice

I would have expected it to just concatenate the arrays.

[skarab42]

I would have expected it to just concatenate the arrays.

In fact I have thought wrongly, I should rather think in terms of types and not values.

MergeDeep<{items: [1, 2, 3, 4, 5, 6]}, {items: ['a', 'b', 'c']}>;
// -> {items: ['a', 'b', 'c']}
// -> {items: string[]}

MergeDeep<{items: [1, 2, 3, 4, 5, 6]}, {items: ['a', 'b', 'c']}, {recurseIntoArrays: true}>;
// -> {items: [1, 2, 3, 4, 5, 6, 'a', 'b', 'c']}
// -> {items: (string|number)[]}

Any suggestions?

[sindresorhus]

In fact I have thought wrongly, I should rather think in terms of types and not values.

I agree with your examples.

---

// -> {items: string[]}

👍

// -> {items: (string|number)[]}

👍

[Uzlopak]

This is really interesting. If you like you could provide some useful typings to @fastify/deepmerge :)

[skarab42]

@sindresorhus If you don't see anything to change, for me it's all good, the refactoring is done ;)

[sindresorhus]

@skarab42 Bump :)

[skarab42]

@sindresorhus I'm working on a better version (I hope), it will take a few more days.

[skarab42]

I am closing this PR due to the new proposal #443.