Add `MergeDeep` type

[skarab42]

@sindresorhus Sorry for the extra work, I'm a bit embarrassed :( I need to review the basics of git and especially conflict resolution. Fixes #443

[sindresorhus]

There are some unresolved comments in the previous pull request:

• https://github.com/sindresorhus/type-fest/pull/443#discussion_r957058900
• https://github.com/sindresorhus/type-fest/pull/443#discussion_r957059558

[sindresorhus]

Don't forget to add Fixes to the pull request description.

[skarab42]

As mentioned here https://github.com/sindresorhus/type-fest/pull/443#discussion_r959225107 I noticed that all the options prefixed with merge-or- are the options that allow you to go deep, so maybe they should be called deep-replace, deep-spread, etc...

[sindresorhus]

I have to do the same thing in the deep version.

Let me know when this is done.

[skarab42]

Let me know when this is done.

@sindresorhus ~~Done, but I think since the test run on [email protected] it fail because it does not incorporate the latest changes on which this type depends aka OmitIndexSignature and PickIndexSignature.~~

Not sure why the tests don't pass in the CI (pass locally).

(P.S: sorry I didn't mean to edit your comment)

[skarab42]

@sindresorhus Merge is broken :( I do a fix!

[skarab42]

@sindresorhus Well, by introducing more and more fixes and options into this type, it becomes too complicated and will quickly become unmaintainable in the long run. I'm thinking of going back to a simpler and more optimised version that doesn't try to cover every edge cases.

[sindresorhus]

I agree

[skarab42]

@sindresorhus In blood and sweat I finally got something I like. Unfortunately I can't make it any simpler without sacrificing features that I consider essential:

• Preservation and merging of index signatures.
• Preservation and normalisation of optional properties.
• Merging of variable length tuples.
• Recursion into arrays/tuples.

[sindresorhus]

This is looking much better. Thanks for working so hard on it.

What's the reasoning for choosing spread as default arrayMergeMode? Isn't replace more expected behavior as that's how it behaves in native JS when you merge (using object.assign or object spread).

I struggle to see when I would use the union arrayMergeMode. Do you have any real-world use-cases in mind?

[skarab42]

Isn't replace more expected behavior as that's how it behaves in native JS when you merge (using object.assign or object spread).

This is indeed the case when you merge two objects, but when you merge two arrays it's the opposite. So I wonder if we shouldn't export a MergeDeepArray type with spread by default and MergeDeepObject with replace by default.

I struggle to see when I would use the union arrayMergeMode.

const initialState = {data: ['a', 'b', 'c']};

// Do the merge
let nextState = {...initialState, data: [1, 2, 3]};

// Error when we try to revert back to inital state, data is typed as number[]
nextState = initialState;

EDIT: In fact I'm not really convinced of the usefulness of union the above example is not realistic as we're going to type the state correctly from the beginning.

[sindresorhus]

This is indeed the case when you merge two objects, but when you merge two arrays it's the opposite. So I wonder if we shouldn't export a MergeDeepArray type with spread by default and MergeDeepObject with replace by default.

Yeah, the important distinction is whether the array is top-level or deep in an object. When it's top-level, it makes to spread, when it's in an object, it makes sense to replace by default.

In fact I'm not really convinced of the usefulness of union the above example is not realistic as we're going to type the state correctly from the beginning.

Maybe drop it for now and see if there's any requests for it?

[skarab42]

@sindresorhus I hope this time it's all right.

[sindresorhus]

So imagine I want to merge:

let a = [
	{
		foo: [1]
	}
];

let b = [
	{
		foo: [true]
	}
];

merge(a, b);

So now by default it will spread the top-level array and replace the a.foo array. Correct?

But if I specify arrayMergeMode: 'spread', does it then apply to both top-level and nested array? If so, I don't think that's desired. Like we discussed, top-level arrays and nested ones have different desired behavior.

I also don't see when you would want arrayMergeMode: 'replace' for top-level arrays. So maybe it's easier to make it so that top-level arrays are always spread and document that arrayMergeMode only applies to nested arrays?

[skarab42]

I also don't see when you would want arrayMergeMode: 'replace' for top-level arrays.

Probably when used in another type.

So maybe it's easier to make it so that top-level arrays are always spread and document that arrayMergeMode only applies to nested arrays?

@sindresorhus Yep right. I added the spreadTopLevelArrays option with a default value of true which is the easiest solution I found without having to create a new branch and duplicate some code.

[sindresorhus]

Probably when used in another type.

Not sure I understand. Do you have a use-case in mind?

I added the spreadTopLevelArrays option with a default value of true which is the easiest solution I found without having to create a new branch and duplicate some code.

My question is, do we even need an option for it?

---

Sorry this is taking long. I'm just trying hard to keep the type as simple as possible and ensure we don't add stuff people don't actually need.

[skarab42]

My question is, do we even need an option for it?

We don't need to make it public, the vast majority of users won't need it (and for the moment I can't find any specific use cases). I can make it static but that won't simplify the type itself, only it signature.

[skarab42]

@sindresorhus Alternatively, we could also mark the option as @internal?

[sindresorhus]

I can make it static but that won't simplify the type itself, only it signature.

👍

[skarab42]

@sindresorhus Done ;)

[sindresorhus]

Awesome work! I'm very happy with what we ended up with ❤️