TehShrike / deepmerge

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

typed deepmerge #198

Open bryan-hunter opened 4 years ago

bryan-hunter commented 4 years ago

https://github.com/TehShrike/deepmerge/issues/179

This deep merges more accurately than https://github.com/TehShrike/deepmerge/pull/181

I added a typescript test to validate some more complex use cases

RebeccaStevens commented 3 years ago

This is good but it's doesn't handle every case yet.

Problems found:

Case 1:

const a = {
  a: 1,
  b: 'abc',
};
const b = {
  b: 2,
};

const r1 = deepmerge(a, b);

r1 is typed as { a: number; b: string | number; } here where it should be typed as { a: number; b: number; }

Case 2:

interface T {
  a: string;
  b?: string;
}
const t1: T = { a: "foo" };
const t2: T = { a: "bar" };

const r2 = deepmerge(t1, t2);

r2 is typed as { a: string; b: string | undefined; } here but it should be typed as T (which is { a: string; b?: string }). This difference in the types is subtle but can be important - With type T, the key b may not exist on the object, but in typeof r2 the key b must exist on the object.

RebeccaStevens commented 3 years ago

I've edited my above comment as I wasn't initially run in strict mode while test (why isn't strict mode on by default???).

Anywho, @TehShrike I think this MR is good enough to merge in as is. There are slight issues with it and improvements that can be made but this type def is a lot better than what's currently on master.