From 11952b672648ab420a853d85bc2a50a41bd1ef0c Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Tue, 7 Sep 2021 23:25:18 -0400 Subject: [PATCH 01/23] Prevent mutable structures when merging Immutable objects --- __tests__/issues.ts | 39 ++++++++++++++++++++++++++++++++++++++- src/functional/merge.js | 14 +++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/__tests__/issues.ts b/__tests__/issues.ts index c57d34d99..58cbb1976 100644 --- a/__tests__/issues.ts +++ b/__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,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, + }, + ], + }) as Map; + 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; + const b = fromJS({ + ch: { code: 8 }, + }); + expect(a.mergeDeep(b).equals(b)).toBe(true); + }); +}); diff --git a/src/functional/merge.js b/src/functional/merge.js index c9f364d6d..41c2a016a 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -1,7 +1,10 @@ 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 isPlainObject from '../utils/isPlainObj'; import shallowCopy from '../utils/shallowCopy'; export function merge(collection, ...sources) { @@ -69,10 +72,19 @@ export function mergeWithSources(collection, sources, merger) { function deepMergerWith(merger) { function deepMerger(oldValue, newValue, key) { return isDataStructure(oldValue) && isDataStructure(newValue) - ? mergeWithSources(oldValue, [newValue], deepMerger) + ? areMergeable(oldValue, newValue) + ? mergeWithSources(oldValue, [newValue], deepMerger) + : newValue : merger ? merger(oldValue, newValue, key) : newValue; } return deepMerger; } + +function areMergeable(oldValue, newValue) { + return !( + (isIndexed(oldValue) || Array.isArray(oldValue)) && + (isKeyed(newValue) || isPlainObject(newValue)) + ); +} From 8e06ea59ba8d63b78cc7730740868fc9c81b9461 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Wed, 8 Sep 2021 10:48:36 -0400 Subject: [PATCH 02/23] Add test for partial conflicts --- __tests__/merge.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/__tests__/merge.ts b/__tests__/merge.ts index cddb3aca0..7fa75ec18 100644 --- a/__tests__/merge.ts +++ b/__tests__/merge.ts @@ -279,4 +279,32 @@ describe('merge', () => { 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; + 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); + }); }); From 6a30077ab921412d9efd954ac2320a35136ed918 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Wed, 8 Sep 2021 11:05:41 -0400 Subject: [PATCH 03/23] Override values consistently --- __tests__/issues.ts | 22 +++++++++++++++++++++- src/functional/merge.js | 12 ++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/__tests__/issues.ts b/__tests__/issues.ts index 58cbb1976..7083a235e 100644 --- a/__tests__/issues.ts +++ b/__tests__/issues.ts @@ -155,6 +155,26 @@ describe('Issue #1475', () => { const b = fromJS({ ch: { code: 8 }, }); - expect(a.mergeDeep(b).equals(b)).toBe(true); + 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; + const objWithArray = fromJS({ + items: [ + { + id: '1', + }, + ], + }); + expect(objWithObj.mergeDeep(objWithArray).equals(objWithArray)).toBe(true); }); }); diff --git a/src/functional/merge.js b/src/functional/merge.js index 41c2a016a..fcf5fbb4f 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -84,7 +84,15 @@ function deepMergerWith(merger) { function areMergeable(oldValue, newValue) { return !( - (isIndexed(oldValue) || Array.isArray(oldValue)) && - (isKeyed(newValue) || isPlainObject(newValue)) + (isIndexedOrArray(oldValue) && isKeyedOrPlainObject(newValue)) || + (isKeyedOrPlainObject(oldValue) && isIndexedOrArray(newValue)) ); } + +function isIndexedOrArray(value) { + return isIndexed(value) || Array.isArray(value); +} + +function isKeyedOrPlainObject(value) { + return isKeyed(value) || isPlainObject(value); +} From 8045f23976ad0ce20209312957515930517151ee Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Thu, 9 Sep 2021 17:16:01 -0400 Subject: [PATCH 04/23] Remove failing test --- __tests__/merge.ts | 65 ---------------------------------------------- 1 file changed, 65 deletions(-) diff --git a/__tests__/merge.ts b/__tests__/merge.ts index 7fa75ec18..112f61608 100644 --- a/__tests__/merge.ts +++ b/__tests__/merge.ts @@ -210,71 +210,6 @@ 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); From 75919934b69aed0d075ed7d19e13cbf91e63b9b1 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Thu, 9 Sep 2021 17:56:18 -0400 Subject: [PATCH 05/23] Use Seq to determine whether they are indexed or keyed --- src/functional/merge.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/functional/merge.js b/src/functional/merge.js index fcf5fbb4f..31ef5e931 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -2,6 +2,7 @@ import { isImmutable } from '../predicates/isImmutable'; import { isIndexed } from '../predicates/isIndexed'; import { isKeyed } from '../predicates/isKeyed'; import { IndexedCollection, KeyedCollection } from '../Collection'; +import { Seq } from '../Seq'; import hasOwnProperty from '../utils/hasOwnProperty'; import isDataStructure from '../utils/isDataStructure'; import isPlainObject from '../utils/isPlainObj'; @@ -82,17 +83,13 @@ function deepMergerWith(merger) { return deepMerger; } -function areMergeable(oldValue, newValue) { +/** + * The data structures are considered not to be mergeable if one of the data structures is indexed and the other is + * keyed. + */ +function areMergeable(oldDataStructure, newDataStructure) { return !( - (isIndexedOrArray(oldValue) && isKeyedOrPlainObject(newValue)) || - (isKeyedOrPlainObject(oldValue) && isIndexedOrArray(newValue)) + (isIndexed(Seq(oldDataStructure)) && isKeyed(Seq(newDataStructure))) || + (isKeyed(Seq(oldDataStructure)) && isIndexed(Seq(newDataStructure))) ); } - -function isIndexedOrArray(value) { - return isIndexed(value) || Array.isArray(value); -} - -function isKeyedOrPlainObject(value) { - return isKeyed(value) || isPlainObject(value); -} From a68ea68a90886794a128ed317c9c28d25a5be5aa Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Thu, 9 Sep 2021 17:58:27 -0400 Subject: [PATCH 06/23] Remove unused import --- src/functional/merge.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/functional/merge.js b/src/functional/merge.js index 31ef5e931..cd2ae2530 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -5,7 +5,6 @@ import { IndexedCollection, KeyedCollection } from '../Collection'; import { Seq } from '../Seq'; import hasOwnProperty from '../utils/hasOwnProperty'; import isDataStructure from '../utils/isDataStructure'; -import isPlainObject from '../utils/isPlainObj'; import shallowCopy from '../utils/shallowCopy'; export function merge(collection, ...sources) { From 6ac0924bd8624f9736d278c81be40214f235084b Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Thu, 9 Sep 2021 18:00:23 -0400 Subject: [PATCH 07/23] Simplify wording --- src/functional/merge.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/functional/merge.js b/src/functional/merge.js index cd2ae2530..e63918ea2 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -83,8 +83,7 @@ function deepMergerWith(merger) { } /** - * The data structures are considered not to be mergeable if one of the data structures is indexed and the other is - * keyed. + * The data structures are considered not to be mergeable if one of them is indexed and the other is keyed. */ function areMergeable(oldDataStructure, newDataStructure) { return !( From 34ca015932193709fb78835c6e7751c051dcb967 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Thu, 9 Sep 2021 19:52:45 -0400 Subject: [PATCH 08/23] Be more strict about mergeability --- src/functional/merge.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/functional/merge.js b/src/functional/merge.js index e63918ea2..92f0056c6 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -1,6 +1,7 @@ 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'; @@ -71,10 +72,10 @@ export function mergeWithSources(collection, sources, merger) { function deepMergerWith(merger) { function deepMerger(oldValue, newValue, key) { - return isDataStructure(oldValue) && isDataStructure(newValue) - ? areMergeable(oldValue, newValue) - ? mergeWithSources(oldValue, [newValue], deepMerger) - : newValue + return isDataStructure(oldValue) && + isDataStructure(newValue) && + areMergeable(oldValue, newValue) + ? mergeWithSources(oldValue, [newValue], deepMerger) : merger ? merger(oldValue, newValue, key) : newValue; @@ -82,12 +83,12 @@ function deepMergerWith(merger) { return deepMerger; } -/** - * The data structures are considered not to be mergeable if one of them is indexed and the other is keyed. - */ function areMergeable(oldDataStructure, newDataStructure) { - return !( - (isIndexed(Seq(oldDataStructure)) && isKeyed(Seq(newDataStructure))) || - (isKeyed(Seq(oldDataStructure)) && isIndexed(Seq(newDataStructure))) + const oldSeq = Seq(oldDataStructure); + const newSeq = Seq(newDataStructure); + return ( + isIndexed(oldSeq) === isIndexed(newSeq) && + isKeyed(oldSeq) === isKeyed(newSeq) && + isSet(oldSeq) === isSet(newSeq) ); } From ab51c3e29f00a998e52f02dce450111e79afd512 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Thu, 9 Sep 2021 20:21:51 -0400 Subject: [PATCH 09/23] Update documentation --- type-definitions/immutable.d.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index 456f2bc81..f468f68c6 100644 --- a/type-definitions/immutable.d.ts +++ b/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 first Collection + * with the second 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. * * * ```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. From 48183f1099be886a8b737c9cae8b22adba6f74e4 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Thu, 9 Sep 2021 20:23:09 -0400 Subject: [PATCH 10/23] Change wording --- type-definitions/immutable.d.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index f468f68c6..e8fbd2d43 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1017,8 +1017,8 @@ declare namespace Immutable { /** * 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 first Collection - * with the second one. + * 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 From 3e9c70390f44dc7e37adeae62c6870203f194d44 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Fri, 10 Sep 2021 14:30:39 -0400 Subject: [PATCH 11/23] Remove isSet check --- src/functional/merge.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/functional/merge.js b/src/functional/merge.js index 92f0056c6..e2a6abaca 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -88,7 +88,6 @@ function areMergeable(oldDataStructure, newDataStructure) { const newSeq = Seq(newDataStructure); return ( isIndexed(oldSeq) === isIndexed(newSeq) && - isKeyed(oldSeq) === isKeyed(newSeq) && - isSet(oldSeq) === isSet(newSeq) + isKeyed(oldSeq) === isKeyed(newSeq) ); } From 050dfd00a7af0c6aa2c6f779f8d1966c26185228 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Fri, 10 Sep 2021 14:33:28 -0400 Subject: [PATCH 12/23] Remove unused import --- src/functional/merge.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/functional/merge.js b/src/functional/merge.js index e2a6abaca..101e55f04 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -1,7 +1,6 @@ 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'; From e912a1b33f7a4658b8ce0202ba3fca8de85103eb Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 16:25:59 -0400 Subject: [PATCH 13/23] Add more test cases --- __tests__/merge.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/__tests__/merge.ts b/__tests__/merge.ts index 112f61608..991518af4 100644 --- a/__tests__/merge.ts +++ b/__tests__/merge.ts @@ -242,4 +242,17 @@ describe('merge', () => { ) ).toBe(true); }); + + it('Map#mergeDeep replaces nested Lists with Map', () => { + const a = Map({ a: List([Map({ x: 1 })]) }); + const b = Map({ a: Map([[0, Map({ y: 2 })]]) }); + // @ts-ignore + expect(a.mergeDeep(b).equals(b)).toBe(true); + }); + + it('functional mergeDeep replaces arrays with Maps', () => { + const a = { a: [{ x: 1 }] }; + const b = Map({ a: Map([[0, Map({ y: 2 })]]) }); + expect(mergeDeep(a, b)).toEqual({ a: Map([[0, Map({ y: 2 })]]) }); + }); }); From cbaa0710eed9ff22d2967420e44d6cfa6347593d Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 17:35:13 -0400 Subject: [PATCH 14/23] Add more test cases --- __tests__/merge.ts | 85 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/__tests__/merge.ts b/__tests__/merge.ts index 991518af4..1c0ec5cac 100644 --- a/__tests__/merge.ts +++ b/__tests__/merge.ts @@ -243,14 +243,89 @@ describe('merge', () => { ).toBe(true); }); - it('Map#mergeDeep replaces nested Lists with Map', () => { - const a = Map({ a: List([Map({ x: 1 })]) }); - const b = Map({ a: Map([[0, Map({ y: 2 })]]) }); - // @ts-ignore + const map = { type: 'Map', value: Map({ b: 5, c: 9 }) }; + const object = { type: 'object', value: { b: 7, d: 12 } }; + const RecordFactory = Record({ a: 1, b: 2 }); + const record = { type: 'Record', value: RecordFactory({ b: 3 }) }; + const list = { type: 'List', value: List(['5']) }; + const array = { type: 'array', value: ['9'] }; + const set = { type: 'Set', value: Set('3') }; + + const incompatibleTypes = [ + [map, list], + [map, array], + [map, set], + [object, list], + [object, array], + [object, set], + [record, list], + [record, array], + [record, set], + [list, set], + ]; + + for (const [ + { type: type1, value: value1 }, + { type: type2, value: value2 }, + ] of incompatibleTypes) { + it(`mergeDeep and Map#mergeDeep replaces ${type1} and ${type2} with each other`, () => { + const aObject = { a: value1 }; + const bObject = { a: value2 }; + expect(mergeDeep(aObject, bObject)).toEqual(bObject); + expect(mergeDeep(bObject, aObject)).toEqual(aObject); + + const aMap = Map({ a: value1 }) as Map; + const bMap = Map({ a: value2 }) as Map; + expect(aMap.mergeDeep(bMap).equals(bMap)).toBe(true); + expect(bMap.mergeDeep(aMap).equals(aMap)).toBe(true); + }); + } + + const compatibleTypesAndResult = [ + [map, object, Map({ b: 7, c: 9, d: 12 })], + [map, record, Map({ a: 1, b: 3, c: 9 })], + [object, map, { b: 5, c: 9, d: 12 }], + [object, record, { a: 1, b: 3, d: 12 }], + [record, map, RecordFactory({ b: 5 })], + [record, object, RecordFactory({ b: 7 })], + [list, array, List(['5', '9'])], + [array, list, ['9', '5']], + [map, { type: 'Map', value: Map({ b: 7 }) }, Map({ b: 7, c: 9 })], + [object, { type: 'object', value: { d: 3 } }, { b: 7, d: 3 }], + [ + record, + { type: 'Record', value: RecordFactory({ a: 3 }) }, + RecordFactory({ a: 3, b: 2 }), + ], + [list, { type: 'List', value: List(['12']) }, List(['5', '12'])], + [array, { type: 'array', value: ['3'] }, ['9', '3']], + [set, { type: 'Set', value: Set(['3', '5']) }, Set(['3', '5'])], + ] as const; + + for (const [ + { type: type1, value: value1 }, + { type: type2, value: value2 }, + result, + ] of compatibleTypesAndResult) { + it(`mergeDeep and Map#mergeDeep merges ${type1} and ${type2}`, () => { + const aObject = { a: value1 }; + const bObject = { a: value2 }; + expect(mergeDeep(aObject, bObject)).toEqual({ a: result }); + + const aMap = Map({ a: value1 }) as Map; + const bMap = Map({ a: value2 }); + expect(aMap.mergeDeep(bMap)).toEqual(Map({ a: result })); + }); + } + + it('Map#mergeDeep replaces nested List with Map and Map with List', () => { + const a = Map({ a: List([Map({ x: 1 })]) }) as Map; + const b = Map({ a: Map([[0, Map({ y: 2 })]]) }) as Map; expect(a.mergeDeep(b).equals(b)).toBe(true); + expect(b.mergeDeep(a).equals(a)).toBe(true); }); - it('functional mergeDeep replaces arrays with Maps', () => { + it('functional mergeDeep replaces nested array with Map', () => { const a = { a: [{ x: 1 }] }; const b = Map({ a: Map([[0, Map({ y: 2 })]]) }); expect(mergeDeep(a, b)).toEqual({ a: Map([[0, Map({ y: 2 })]]) }); From 25896fa3c097865975a427ef6268d8fe8566b76c Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:16:41 -0400 Subject: [PATCH 15/23] Update documentation --- type-definitions/immutable.d.ts | 44 +++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index e8fbd2d43..990830821 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1015,14 +1015,17 @@ declare namespace Immutable { ): this; /** - * 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 - * a deeper level. + * Like `merge()`, but when two collections of similar types are encountered + * when merging two values, it merges them as well, recursing deeply through + * the nested data. Collections are considered to be of similar types (and + * thus will be merged) based on whether they are keyed (e.g., `Map`s, + * `Record`s, and objects), indexed (e.g., `List`s and arrays), or set-like + * (e.g., `Set`s). If they are not of similar types, `mergeDeep` will + * replace the existing collection with the collection being merged in. This + * behavior can be customized by using `mergeDeepWith()`. + * + * Note: Indexed and set-like collections are merged using `concat`/`union` + * and therefore do not recurse. * * * ```js @@ -1044,9 +1047,9 @@ declare namespace Immutable { ): this; /** - * Like `mergeDeep()`, but when two non-Collections or two Collections of - * different types conflict, it uses the `merger` function to determine the - * resulting value. + * Like `mergeDeep()`, but when two non-collections or collections of + * different types are encountered when merging two values, it uses the + * `merger` function to determine the resulting value. * * * ```js @@ -5537,8 +5540,17 @@ declare namespace Immutable { ): C; /** - * Returns a copy of the collection with the remaining collections merged in - * deeply (recursively). + * Like `merge()`, but when two collections of similar types are encountered + * when merging two values, it merges them as well, recursing deeply through + * the nested data. Collections are considered to be of similar types (and + * thus will be merged) based on whether they are keyed (e.g., `Map`s, + * `Record`s, and objects), indexed (e.g., `List`s and arrays), or set-like + * (e.g., `Set`s). If they are not of similar types, `mergeDeep` will replace + * the existing collection with the collection being merged in. This behavior + * can be customized by using `mergeDeepWith()`. + * + * Note: Indexed and set-like collections are merged using `concat`/`union` + * and therefore do not recurse. * * A functional alternative to `collection.mergeDeep()` which will also work * with plain Objects and Arrays. @@ -5561,9 +5573,9 @@ declare namespace Immutable { ): C; /** - * Returns a copy of the collection with the remaining collections merged in - * deeply (recursively), calling the `merger` function whenever an existing - * value or incompatible data structures are encountered. + * Like `mergeDeep()`, but when two non-collections or collections of + * different types are encountered when merging two values, it uses the + * `merger` function to determine the resulting value. * * A functional alternative to `collection.mergeDeepWith()` which will also * work with plain Objects and Arrays. From b526da3f1b44a637da55373d421c307e914654a4 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:18:50 -0400 Subject: [PATCH 16/23] Update documentation --- type-definitions/immutable.d.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index 990830821..4c9d07dab 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1024,8 +1024,8 @@ declare namespace Immutable { * replace the existing collection with the collection being merged in. This * behavior can be customized by using `mergeDeepWith()`. * - * Note: Indexed and set-like collections are merged using `concat`/`union` - * and therefore do not recurse. + * Note: Indexed and set-like collections are merged using + * `concat()`/`union()` and therefore do not recurse. * * * ```js @@ -5549,8 +5549,8 @@ declare namespace Immutable { * the existing collection with the collection being merged in. This behavior * can be customized by using `mergeDeepWith()`. * - * Note: Indexed and set-like collections are merged using `concat`/`union` - * and therefore do not recurse. + * Note: Indexed and set-like collections are merged using + * `concat()`/`union()` and therefore do not recurse. * * A functional alternative to `collection.mergeDeep()` which will also work * with plain Objects and Arrays. From 63f4b803842141e3dea096b485c528413d2768f5 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:24:32 -0400 Subject: [PATCH 17/23] Update documentation --- type-definitions/immutable.d.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index 4c9d07dab..a52fb7c61 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1048,8 +1048,8 @@ declare namespace Immutable { /** * Like `mergeDeep()`, but when two non-collections or collections of - * different types are encountered when merging two values, it uses the - * `merger` function to determine the resulting value. + * types that are not similar are encountered when merging two values, it + * uses the `merger` function to determine the resulting value. * * * ```js @@ -5574,8 +5574,8 @@ declare namespace Immutable { /** * Like `mergeDeep()`, but when two non-collections or collections of - * different types are encountered when merging two values, it uses the - * `merger` function to determine the resulting value. + * types that are not similar are encountered when merging two values, it uses + * the `merger` function to determine the resulting value. * * A functional alternative to `collection.mergeDeepWith()` which will also * work with plain Objects and Arrays. From 1ab0cd9aeb6027ed85caeeac967789b1ffa8551a Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:36:34 -0400 Subject: [PATCH 18/23] Update documentation --- type-definitions/immutable.d.ts | 36 ++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index a52fb7c61..f3df705a5 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1018,11 +1018,12 @@ declare namespace Immutable { * Like `merge()`, but when two collections of similar types are encountered * when merging two values, it merges them as well, recursing deeply through * the nested data. Collections are considered to be of similar types (and - * thus will be merged) based on whether they are keyed (e.g., `Map`s, - * `Record`s, and objects), indexed (e.g., `List`s and arrays), or set-like - * (e.g., `Set`s). If they are not of similar types, `mergeDeep` will - * replace the existing collection with the collection being merged in. This - * behavior can be customized by using `mergeDeepWith()`. + * thus will be merged) if they both fall into one of three categories: + * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and + * arrays), or set-like (e.g., `Set`s). If they fall into separate + * categories, `mergeDeep` will replace the existing collection with the + * collection being merged in. This behavior can be customized by using + * `mergeDeepWith()`. * * Note: Indexed and set-like collections are merged using * `concat()`/`union()` and therefore do not recurse. @@ -1047,9 +1048,10 @@ declare namespace Immutable { ): this; /** - * Like `mergeDeep()`, but when two non-collections or collections of - * types that are not similar are encountered when merging two values, it - * uses the `merger` function to determine the resulting value. + * Like `mergeDeep()`, but when two non-collections or collections that fall + * into separate categories (either keyed, indexed, or set-like) are + * encountered when merging two values, it uses the `merger` function to + * determine the resulting value. * * * ```js @@ -5543,11 +5545,12 @@ declare namespace Immutable { * Like `merge()`, but when two collections of similar types are encountered * when merging two values, it merges them as well, recursing deeply through * the nested data. Collections are considered to be of similar types (and - * thus will be merged) based on whether they are keyed (e.g., `Map`s, - * `Record`s, and objects), indexed (e.g., `List`s and arrays), or set-like - * (e.g., `Set`s). If they are not of similar types, `mergeDeep` will replace - * the existing collection with the collection being merged in. This behavior - * can be customized by using `mergeDeepWith()`. + * thus will be merged) if they both fall into one of three categories: + * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and + * arrays), or set-like (e.g., `Set`s). If they fall into separate + * categories, `mergeDeep` will replace the existing collection with the + * collection being merged in. This behavior can be customized by using + * `mergeDeepWith()`. * * Note: Indexed and set-like collections are merged using * `concat()`/`union()` and therefore do not recurse. @@ -5573,9 +5576,10 @@ declare namespace Immutable { ): C; /** - * Like `mergeDeep()`, but when two non-collections or collections of - * types that are not similar are encountered when merging two values, it uses - * the `merger` function to determine the resulting value. + * Like `mergeDeep()`, but when two non-collections or collections that fall + * into separate categories (either keyed, indexed, or set-like) are + * encountered when merging two values, it uses the `merger` function to + * determine the resulting value. * * A functional alternative to `collection.mergeDeepWith()` which will also * work with plain Objects and Arrays. From 31e46bd2ca5a13ece6f95fe1f66fbb8820684c60 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:44:22 -0400 Subject: [PATCH 19/23] Update documentation --- type-definitions/immutable.d.ts | 54 +++++++++++++++++---------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index f3df705a5..291d92183 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1015,15 +1015,15 @@ declare namespace Immutable { ): this; /** - * Like `merge()`, but when two collections of similar types are encountered - * when merging two values, it merges them as well, recursing deeply through - * the nested data. Collections are considered to be of similar types (and - * thus will be merged) if they both fall into one of three categories: - * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and - * arrays), or set-like (e.g., `Set`s). If they fall into separate - * categories, `mergeDeep` will replace the existing collection with the - * collection being merged in. This behavior can be customized by using - * `mergeDeepWith()`. + * Like `merge()`, but when two compatible collections of similar types are + * encountered when merging values, it merges them as well, recursing + * deeply through the nested data. Two collections are considered to be + * compatible (and thus will be merged together) if they both fall into one + * of three categories: keyed (e.g., `Map`s, `Record`s, and objects), + * indexed (e.g., `List`s and arrays), or set-like (e.g., `Set`s). If they + * fall into separate categories, `mergeDeep` will replace the existing + * collection with the collection being merged in. This behavior can be + * customized by using `mergeDeepWith()`. * * Note: Indexed and set-like collections are merged using * `concat()`/`union()` and therefore do not recurse. @@ -1048,10 +1048,11 @@ declare namespace Immutable { ): this; /** - * Like `mergeDeep()`, but when two non-collections or collections that fall - * into separate categories (either keyed, indexed, or set-like) are - * encountered when merging two values, it uses the `merger` function to - * determine the resulting value. + * Like `mergeDeep()`, but when two non-collections or incompatible + * collections are encountered when merging values, it uses the `merger` + * function to determine the resulting value. Collections are considered + * incompatible if they fall into separate categories between keyed, + * indexed, and set-like. * * * ```js @@ -5542,15 +5543,15 @@ declare namespace Immutable { ): C; /** - * Like `merge()`, but when two collections of similar types are encountered - * when merging two values, it merges them as well, recursing deeply through - * the nested data. Collections are considered to be of similar types (and - * thus will be merged) if they both fall into one of three categories: - * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and - * arrays), or set-like (e.g., `Set`s). If they fall into separate - * categories, `mergeDeep` will replace the existing collection with the - * collection being merged in. This behavior can be customized by using - * `mergeDeepWith()`. + * Like `merge()`, but when two compatible collections of similar types are + * encountered when merging values, it merges them as well, recursing + * deeply through the nested data. Two collections are considered to be + * compatible (and thus will be merged together) if they both fall into one + * of three categories: keyed (e.g., `Map`s, `Record`s, and objects), + * indexed (e.g., `List`s and arrays), or set-like (e.g., `Set`s). If they + * fall into separate categories, `mergeDeep` will replace the existing + * collection with the collection being merged in. This behavior can be + * customized by using `mergeDeepWith()`. * * Note: Indexed and set-like collections are merged using * `concat()`/`union()` and therefore do not recurse. @@ -5576,10 +5577,11 @@ declare namespace Immutable { ): C; /** - * Like `mergeDeep()`, but when two non-collections or collections that fall - * into separate categories (either keyed, indexed, or set-like) are - * encountered when merging two values, it uses the `merger` function to - * determine the resulting value. + * Like `mergeDeep()`, but when two non-collections or incompatible + * collections are encountered when merging values, it uses the `merger` + * function to determine the resulting value. Collections are considered + * incompatible if they fall into separate categories between keyed, + * indexed, and set-like. * * A functional alternative to `collection.mergeDeepWith()` which will also * work with plain Objects and Arrays. From 6e3be746c7f44e2e7aa5097fc65aa80c068ebaf7 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:46:08 -0400 Subject: [PATCH 20/23] Update documentation --- type-definitions/immutable.d.ts | 36 ++++++++++++++++----------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index 291d92183..cbc90f45b 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1015,15 +1015,15 @@ declare namespace Immutable { ): this; /** - * Like `merge()`, but when two compatible collections of similar types are - * encountered when merging values, it merges them as well, recursing - * deeply through the nested data. Two collections are considered to be - * compatible (and thus will be merged together) if they both fall into one - * of three categories: keyed (e.g., `Map`s, `Record`s, and objects), - * indexed (e.g., `List`s and arrays), or set-like (e.g., `Set`s). If they - * fall into separate categories, `mergeDeep` will replace the existing - * collection with the collection being merged in. This behavior can be - * customized by using `mergeDeepWith()`. + * Like `merge()`, but when two compatible collections are encountered when + * merging values, it merges them as well, recursing deeply through the + * nested data. Two collections are considered to be compatible (and thus + * will be merged together) if they both fall into one of three categories: + * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and + * arrays), or set-like (e.g., `Set`s). If they fall into separate + * categories, `mergeDeep` will replace the existing collection with the + * collection being merged in. This behavior can be customized by using + * `mergeDeepWith()`. * * Note: Indexed and set-like collections are merged using * `concat()`/`union()` and therefore do not recurse. @@ -5543,15 +5543,15 @@ declare namespace Immutable { ): C; /** - * Like `merge()`, but when two compatible collections of similar types are - * encountered when merging values, it merges them as well, recursing - * deeply through the nested data. Two collections are considered to be - * compatible (and thus will be merged together) if they both fall into one - * of three categories: keyed (e.g., `Map`s, `Record`s, and objects), - * indexed (e.g., `List`s and arrays), or set-like (e.g., `Set`s). If they - * fall into separate categories, `mergeDeep` will replace the existing - * collection with the collection being merged in. This behavior can be - * customized by using `mergeDeepWith()`. + * Like `merge()`, but when two compatible collections are encountered when + * merging values, it merges them as well, recursing deeply through the + * nested data. Two collections are considered to be compatible (and thus + * will be merged together) if they both fall into one of three categories: + * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and + * arrays), or set-like (e.g., `Set`s). If they fall into separate + * categories, `mergeDeep` will replace the existing collection with the + * collection being merged in. This behavior can be customized by using + * `mergeDeepWith()`. * * Note: Indexed and set-like collections are merged using * `concat()`/`union()` and therefore do not recurse. From 5b439725c609e5b8784f692d39877eb29c6eae43 Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:50:24 -0400 Subject: [PATCH 21/23] Update documentation --- type-definitions/immutable.d.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index cbc90f45b..c0adc16ed 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1015,11 +1015,11 @@ declare namespace Immutable { ): this; /** - * Like `merge()`, but when two compatible collections are encountered when - * merging values, it merges them as well, recursing deeply through the - * nested data. Two collections are considered to be compatible (and thus - * will be merged together) if they both fall into one of three categories: - * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and + * Like `merge()`, but when two compatible collections are encountered with + * the same key, it merges them as well, recursing deeply through the nested + * data. Two collections are considered to be compatible (and thus will be + * merged together) if they both fall into one of three categories: keyed + * (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and * arrays), or set-like (e.g., `Set`s). If they fall into separate * categories, `mergeDeep` will replace the existing collection with the * collection being merged in. This behavior can be customized by using @@ -5543,11 +5543,11 @@ declare namespace Immutable { ): C; /** - * Like `merge()`, but when two compatible collections are encountered when - * merging values, it merges them as well, recursing deeply through the - * nested data. Two collections are considered to be compatible (and thus - * will be merged together) if they both fall into one of three categories: - * keyed (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and + * Like `merge()`, but when two compatible collections are encountered with + * the same key, it merges them as well, recursing deeply through the nested + * data. Two collections are considered to be compatible (and thus will be + * merged together) if they both fall into one of three categories: keyed + * (e.g., `Map`s, `Record`s, and objects), indexed (e.g., `List`s and * arrays), or set-like (e.g., `Set`s). If they fall into separate * categories, `mergeDeep` will replace the existing collection with the * collection being merged in. This behavior can be customized by using From 2bd33cb2f008c66ddfd16500a64fec4fa293387b Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Sun, 12 Sep 2021 18:53:26 -0400 Subject: [PATCH 22/23] Update documentation --- type-definitions/immutable.d.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index c0adc16ed..187e3d41b 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -1049,7 +1049,7 @@ declare namespace Immutable { /** * Like `mergeDeep()`, but when two non-collections or incompatible - * collections are encountered when merging values, it uses the `merger` + * collections are encountered at the same key, it uses the `merger` * function to determine the resulting value. Collections are considered * incompatible if they fall into separate categories between keyed, * indexed, and set-like. @@ -5578,10 +5578,9 @@ declare namespace Immutable { /** * Like `mergeDeep()`, but when two non-collections or incompatible - * collections are encountered when merging values, it uses the `merger` - * function to determine the resulting value. Collections are considered - * incompatible if they fall into separate categories between keyed, - * indexed, and set-like. + * collections are encountered at the same key, it uses the `merger` function + * to determine the resulting value. Collections are considered incompatible + * if they fall into separate categories between keyed, indexed, and set-like. * * A functional alternative to `collection.mergeDeepWith()` which will also * work with plain Objects and Arrays. From be3e91fafeee59d31f05bf58090902e0bca1f91e Mon Sep 17 00:00:00 2001 From: Nathan Bierema Date: Mon, 13 Sep 2021 16:04:30 -0400 Subject: [PATCH 23/23] Add comments --- src/functional/merge.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/functional/merge.js b/src/functional/merge.js index 101e55f04..21fbd471b 100644 --- a/src/functional/merge.js +++ b/src/functional/merge.js @@ -82,9 +82,16 @@ function deepMergerWith(merger) { return deepMerger; } +/** + * It's unclear what the desired behavior is for merging two collections that + * fall into separate categories between keyed, indexed, or set-like, so we only + * consider them mergeable if they fall into the same category. + */ function areMergeable(oldDataStructure, newDataStructure) { const oldSeq = Seq(oldDataStructure); const newSeq = Seq(newDataStructure); + // This logic assumes that a sequence can only fall into one of the three + // categories mentioned above (since there's no `isSetLike()` method). return ( isIndexed(oldSeq) === isIndexed(newSeq) && isKeyed(oldSeq) === isKeyed(newSeq)