Prevent mutable structures when merging Immutable objects

__tests__/issues.ts

@@ -8,7 +8,15 @@
 ///<reference path='../resources/jest.d.ts'/>
 
 declare var Symbol: any;
-import { List, OrderedMap, OrderedSet, Record, Seq, Set } from 'immutable';
+import {
+  fromJS,
+  List,
+  OrderedMap,
+  OrderedSet,
+  Record,
+  Seq,
+  Set,
+} from 'immutable';
 
 describe('Issue #1175', () => {
   it('invalid hashCode() response should not infinitly recurse', () => {
@@ -131,3 +139,31 @@ 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,
+        },
+      ],
+    });
+    const b = fromJS({
+      ch: {
+        code: 8,
+      },
+    });
+    expect(a.mergeDeep(b)).toBe(a);
+  });
+
+  it('simple case should return first value on mergeDeep when types are incompatible', () => {
+    const a = fromJS({
+      ch: [],
+    });
+    const b = fromJS({
+      ch: { code: 8 },
+    });
+    expect(a.mergeDeep(b)).toBe(a);
+  });
+});

__tests__/merge.ts

@@ -283,4 +283,30 @@ describe('merge', () => {
     const Sizable = Record({ size: 0 });
     expect(Sizable().merge({ size: 123 }).size).toBe(123);
   });
+
+  const incompatibleMerges = [
+    ['Map', 'List', Map({ foo: 'bar' }), List(['bar'])],
+    ['Map', 'array', Map({ foo: 'bar' }), ['bar']],
+    ['object', 'List', { foo: 'bar' }, List(['bar'])],
+    ['object', 'array', { foo: 'bar' }, ['bar']],
+  ];
+
+  incompatibleMerges.forEach(
+    ([firstName, secondName, firstValue, secondValue]) => {
+      it(`returns first value or throws Error when called with ${firstName} and ${secondName}`, () => {
+        try {
+          expect(merge(firstValue, secondValue)).toBe(firstValue);
+        } catch {
+          // Throwing an Error is also acceptable in these cases
+        }
+      });
+      it(`returns first value or throws Error when called with ${secondName} and ${firstName}`, () => {
+        try {
+          expect(merge(secondValue, firstValue)).toBe(secondValue);
+        } catch {
+          // Throwing an Error is also acceptable in these cases
+        }
+      });
+    }
+  );
 });

src/List.js

@@ -18,6 +18,7 @@ import {
   resolveBegin,
   resolveEnd,
 } from './TrieUtils';
+import { isKeyed } from './predicates/isKeyed';
 import { IS_LIST_SYMBOL, isList } from './predicates/isList';
 import { IndexedCollection } from './Collection';
 import { hasIterator, Iterator, iteratorValue, iteratorDone } from './Iterator';
@@ -32,6 +33,7 @@ import { asMutable } from './methods/asMutable';
 import { asImmutable } from './methods/asImmutable';
 import { wasAltered } from './methods/wasAltered';
 import assertNotInfinite from './utils/assertNotInfinite';
+import isPlainObject from './utils/isPlainObj';
 
 export class List extends IndexedCollection {
   // @pragma Construction
@@ -148,10 +150,16 @@ export class List extends IndexedCollection {
     const seqs = [];
     for (let i = 0; i < arguments.length; i++) {
       const argument = arguments[i];
+      const argumentHasIterator =
+        typeof argument !== 'string' && hasIterator(argument);
+      if (
+        isKeyed(argument) ||
+        (!argumentHasIterator && isPlainObject(argument))
+      ) {
+        continue;
+      }
       const seq = IndexedCollection(
-        typeof argument !== 'string' && hasIterator(argument)
-          ? argument
-          : [argument]
+        argumentHasIterator ? argument : [argument]
       );
       if (seq.size !== 0) {
         seqs.push(seq);

src/functional/merge.js

@@ -6,10 +6,13 @@
  */
 
 import { isImmutable } from '../predicates/isImmutable';
+import { isIndexed } from '../predicates/isIndexed';
+import { isKeyed } from '../predicates/isKeyed';
 import { IndexedCollection, KeyedCollection } from '../Collection';
 import hasOwnProperty from '../utils/hasOwnProperty';
 import isDataStructure from '../utils/isDataStructure';
 import shallowCopy from '../utils/shallowCopy';
+import isPlainObject from '../utils/isPlainObj';
 
 export function merge(collection, ...sources) {
   return mergeWithSources(collection, sources);
@@ -45,6 +48,7 @@ export function mergeWithSources(collection, sources, merger) {
       : collection.concat(...sources);
   }
   const isArray = Array.isArray(collection);
+  const isObject = !isArray;
   let merged = collection;
   const Collection = isArray ? IndexedCollection : KeyedCollection;
   const mergeItem = isArray
@@ -68,7 +72,14 @@ export function mergeWithSources(collection, sources, merger) {
         }
       };
   for (let i = 0; i < sources.length; i++) {
-    Collection(sources[i]).forEach(mergeItem);
+    const source = sources[i];
+    if (
+      (isArray && (isPlainObject(source) || isKeyed(source))) ||
+      (isObject && (Array.isArray(source) || isIndexed(source)))
+    ) {
+      continue;
+    }
+    Collection(source).forEach(mergeItem);
   }
   return merged;
 }