Prevent mutable structures when merging Immutable objects using mergeDeep

__tests__/issues.ts

@@ -1,4 +1,13 @@
-import { List, OrderedMap, OrderedSet, Record, Seq, Set } from 'immutable';
+import {
+  fromJS,
+  List,
+  Map,
+  OrderedMap,
+  OrderedSet,
+  Record,
+  Seq,
+  Set,
+} from 'immutable';
 
 describe('Issue #1175', () => {
   it('invalid hashCode() response should not infinitly recurse', () => {
@@ -121,3 +130,51 @@ describe('Issue #1785', () => {
 
   expect(emptyRecord.merge({ id: 1 })).toBe(emptyRecord);
 });
+
+describe('Issue #1475', () => {
+  it('complex case should return first value on mergeDeep when types are incompatible', () => {
+    const a = fromJS({
+      ch: [
+        {
+          code: 8,
+        },
+      ],
+    }) as Map<unknown, unknown>;
+    const b = fromJS({
+      ch: {
+        code: 8,
+      },
+    });
+    expect(a.mergeDeep(b).equals(b)).toBe(true);
+  });
+
+  it('simple case should return first value on mergeDeep when types are incompatible', () => {
+    const a = fromJS({
+      ch: [],
+    }) as Map<unknown, unknown>;
+    const b = fromJS({
+      ch: { code: 8 },
+    });
+    expect(a.merge(b).equals(b)).toBe(true);
+  });
+});
+
+describe('Issue #1719', () => {
+  it('mergeDeep() should overwrite when types conflict', () => {
+    const objWithObj = fromJS({
+      items: {
+        '1': {
+          id: '1',
+        },
+      },
+    }) as Map<unknown, unknown>;
+    const objWithArray = fromJS({
+      items: [
+        {
+          id: '1',
+        },
+      ],
+    });
+    expect(objWithObj.mergeDeep(objWithArray).equals(objWithArray)).toBe(true);
+  });
+});

__tests__/merge.ts

@@ -210,73 +210,36 @@ describe('merge', () => {
     expect(merge(a, [], [])).toBe(a);
   });
 
-  it('mergeDeep with tuple Symbol keys', () => {
-    const a = Symbol('a');
-    const b = Symbol('b');
-    const c = Symbol('c');
-    const d = Symbol('d');
-    const e = Symbol('e');
-    const f = Symbol('f');
-    const g = Symbol('g');
-
-    // Note the use of nested Map constructors, Map() does not do a deep conversion!
-    const m1 = Map([
-      [
-        a,
-        Map([
-          [
-            b,
-            Map([
-              [c, 1],
-              [d, 2],
-            ]),
-          ],
-        ]),
-      ],
-    ]);
-
-    // mergeDeep can be directly given a nested set of `Iterable<[K, V]>`
-    const merged = m1.mergeDeep([
-      // @ts-ignore (Type definition limitation)
-      [
-        a,
-        [
-          [
-            b,
-            [
-              [c, 10],
-              [e, 20],
-              [f, 30],
-              [g, 40],
-            ],
-          ],
-        ],
-      ],
-    ]);
-
-    expect(merged).toEqual(
-      Map([
-        [
-          a,
-          Map([
-            [
-              b,
-              Map([
-                [c, 10],
-                [d, 2],
-                [e, 20],
-                [f, 30],
-                [g, 40],
-              ]),
-            ],
-          ]),
-        ],
-      ])
-    );
-  });
-
   it('merges records with a size property set to 0', () => {
     const Sizable = Record({ size: 0 });
     expect(Sizable().merge({ size: 123 }).size).toBe(123);
   });
+
+  it('mergeDeep merges partial conflicts', () => {
+    const a = fromJS({
+      ch: [
+        {
+          code: 8,
+        },
+      ],
+      banana: 'good',
+    }) as Map<unknown, unknown>;
+    const b = fromJS({
+      ch: {
+        code: 8,
+      },
+      apple: 'anti-doctor',
+    });
+    expect(
+      a.mergeDeep(b).equals(
+        fromJS({
+          ch: {
+            code: 8,
+          },
+          apple: 'anti-doctor',
+          banana: 'good',
+        })
+      )
+    ).toBe(true);
+  });
 });

src/functional/merge.js

@@ -1,5 +1,9 @@
 import { isImmutable } from '../predicates/isImmutable';
+import { isIndexed } from '../predicates/isIndexed';
+import { isKeyed } from '../predicates/isKeyed';
+import { isSet } from '../predicates/isSet';
 import { IndexedCollection, KeyedCollection } from '../Collection';
+import { Seq } from '../Seq';
 import hasOwnProperty from '../utils/hasOwnProperty';
 import isDataStructure from '../utils/isDataStructure';
 import shallowCopy from '../utils/shallowCopy';
@@ -68,11 +72,23 @@ export function mergeWithSources(collection, sources, merger) {
 
 function deepMergerWith(merger) {
   function deepMerger(oldValue, newValue, key) {
-    return isDataStructure(oldValue) && isDataStructure(newValue)
+    return isDataStructure(oldValue) &&
+      isDataStructure(newValue) &&
+      areMergeable(oldValue, newValue)
       ? mergeWithSources(oldValue, [newValue], deepMerger)
       : merger
       ? merger(oldValue, newValue, key)
       : newValue;
   }
   return deepMerger;
 }
+
+function areMergeable(oldDataStructure, newDataStructure) {
+  const oldSeq = Seq(oldDataStructure);
+  const newSeq = Seq(newDataStructure);
+  return (
+    isIndexed(oldSeq) === isIndexed(newSeq) &&
+    isKeyed(oldSeq) === isKeyed(newSeq) &&
+    isSet(oldSeq) === isSet(newSeq)
+  );
+}

type-definitions/immutable.d.ts

@@ -1015,8 +1015,10 @@ declare namespace Immutable {
     ): this;
 
     /**
-     * Like `merge()`, but when two Collections conflict, it merges them as well,
-     * recursing deeply through the nested data.
+     * Like `merge()`, but when two Collections of the same type conflict, it
+     * merges them as well, recursing deeply through the nested data. If two
+     * Collections of different types conflict, it replaces the existing
+     * Collection with the new one.
      *
      * Note: Values provided to `merge` are shallowly converted before being
      * merged. No nested values are altered unless they will also be merged at
@@ -1042,8 +1044,9 @@ declare namespace Immutable {
     ): this;
 
     /**
-     * Like `mergeDeep()`, but when two non-Collections conflict, it uses the
-     * `merger` function to determine the resulting value.
+     * Like `mergeDeep()`, but when two non-Collections or two Collections of
+     * different types conflict, it uses the `merger` function to determine the
+     * resulting value.
      *
      * <!-- runkit:activate -->
      * ```js
@@ -5560,7 +5563,7 @@ declare namespace Immutable {
   /**
    * Returns a copy of the collection with the remaining collections merged in
    * deeply (recursively), calling the `merger` function whenever an existing
-   * value is encountered.
+   * value or incompatible data structures are encountered.
    *
    * A functional alternative to `collection.mergeDeepWith()` which will also
    * work with plain Objects and Arrays.