8000 BREAKING: concat Lists when merging deeply by leebyron · Pull Request #1344 · immutable-js/immutable-js · GitHub
[go: up one dir, main page]
Skip to content

BREAKING: concat Lists when merging deeply #1344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 5, 2017
Merged

BREAKING: concat Lists when merging deeply #1344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions __tests__/merge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

///<reference path='../resources/jest.d.ts'/>

import { fromJS, is, List, Map } from '../';
import { fromJS, is, List, Map, Set } from '../';

describe('merge', () => {
it('merges two maps', () => {
Expand Down Expand Up @@ -108,7 +108,15 @@ describe('merge', () => {
expect(m1.mergeDeep(m2).get('a')).toEqual(Map({x: 1, y: 1, z: 10}));
});

it('merges map entries with Vector values', () => {
it('merges map enties with List and Set values', () => {
const initial = Map({a: Map({x: 10, y: 20}), b: List([1, 2, 3]), c: Set([1, 2, 3])});
const additions = Map({a: Map({y: 50, z: 100}), b: List([4, 5, 6]), c: Set([4, 5, 6])});
expect(initial.mergeDeep(additions)).toEqual(
Map({a: Map({x: 10, y: 50, z: 100}), b: List([1, 2, 3, 4, 5, 6]), c: Set([1, 2, 3, 4, 5, 6])}),
);
});

it('merges map entries with new values', () => {
const initial = Map({a: List([1])});

// Note: merge and mergeDeep do not deeply coerce values, they only merge
Expand All @@ -126,14 +134,14 @@ describe('merge', () => {
});

it('maintains JS values inside immutable collections', () => {
let m1 = fromJS({a: {b: [{imm: 'map'}]}});
let m1 = fromJS({a: {b: {imm: 'map'}}});
let m2 = m1.mergeDeep(
Map({a: Map({b: List.of( {plain: 'obj'} )})}),
Map({a: Map({b: {plain: 'obj'} })}),
);

expect(m1.getIn(['a', 'b', 0])).toEqual(Map([['imm', 'map']]));
expect(m1.getIn(['a', 'b'])).toEqual(Map([['imm', 'map']]));
// However mergeDeep will merge that value into the inner Map
expect(m2.getIn(['a', 'b', 0])).toEqual(Map({imm: 'map', plain: 'obj'}));
expect(m2.getIn(['a', 'b'])).toEqual(Map({imm: 'map', plain: 'obj'}));
});

});
39 changes: 3 additions & 36 deletions src/List.js
View file
Open in desktop
< 8000 td class="blob-code blob-code-deletion js-file-line"> return mergeIntoListWith(this, merger, iters);
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@ import {
resolveEnd
} from './TrieUtils';
import { IndexedCollection } from './Collection';
import {
MapPrototype,
mergeIntoCollectionWith,
deepMerger,
deepMergerWith
} from './Map';
import { MapPrototype } from './Map';
import { Iterator, iteratorValue, iteratorDone } from './Iterator';

import assertNotInfinite from './utils/assertNotInfinite';
Expand Down Expand Up @@ -140,20 +135,8 @@ export class List extends IndexedCollection {

// @pragma Composition

merge(/*...iters*/) {
return mergeIntoListWith(this, undefined, arguments);
}

mergeWith(merger, ...iters) {
}

mergeDeep(/*...iters*/) {
return mergeIntoListWith(this, deepMerger, arguments);
}

mergeDeepWith(merger, ...iters) {
return mergeIntoListWith(this, deepMergerWith(merger), iters);
merge(/*...collections*/) {
return this.concat.apply(this, arguments);
}

setSize(size) {
Expand Down Expand Up @@ -652,22 +635,6 @@ function setListBounds(list, begin, end) {
return makeList(newOrigin, newCapacity, newLevel, newRoot, newTail);
}

function mergeIntoListWith(list, merger, collections) {
const iters = [];
let maxSize = 0;
for (let ii = 0; ii < collections.length; ii++) {
const iter = IndexedCollection(collections[ii]);
if (iter.size > maxSize) {
maxSize = iter.size;
}
iters.push(iter);
}
if (maxSize > list.size) {
list = list.setSize(maxSize);
}
return mergeIntoCollectionWith(list, merger, iters);
}

function getTailOffset(size) {
return size < SIZE ? 0 : ((size - 1) >>> SHIFT) << SHIFT;
}
24 changes: 11 additions & 13 deletions src/Map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class Map extends KeyedCollection {
}

mergeDeep(/*...iters*/) {
return mergeIntoMapWith(this, deepMerger, arguments);
return mergeIntoMapWith(this, deepMergerWith(alwaysNewVal), arguments);
}

mergeDeepWith(merger, ...iters) {
Expand Down Expand Up @@ -839,21 +839,19 @@ function mergeIntoMapWith(map, merger, collections) {
return mergeIntoCollectionWith(map, merger, iters);
}

export function deepMerger(oldVal, newVal) {
return newVal && typeof newVal === 'object' && oldVal && oldVal.mergeDeep
? oldVal.mergeDeep(newVal)
: is(oldVal, newVal) ? oldVal : newVal;
function alwaysNewVal(oldVal, newVal) {
return newVal;
}

export function deepMergerWith(merger) {
return (oldVal, newVal, key) => {
if (
newVal &&
typeof newVal === 'object' &&
oldVal &&
oldVal.mergeDeepWith
) {
return oldVal.mergeDeepWith(merger, newVal);
return function(oldVal, newVal, key) {
if (oldVal && newVal && typeof newVal === 'object') {
if (oldVal.mergeDeepWith) {
return oldVal.mergeDeepWith(merger, newVal);
}
if (oldVal.merge) {
return oldVal.merge(newVal);
}
8000 }
const nextValue = merger(oldVal, newVal, key);
return is(oldVal, nextValue) ? oldVal : nextValue;
Expand Down
11 changes: 1 addition & 10 deletions src/Set.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,6 @@ export class Set extends SetCollection {
});
}

merge() {
return this.union.apply(this, arguments);
}

mergeWith(merger, ...iters) {
return this.union.apply(this, iters);
}

sort(comparator) {
// Late binding
return OrderedSet(sortFactory(this, comparator));
Expand Down Expand Up @@ -186,8 +178,7 @@ const IS_SET_SENTINEL = '@@__IMMUTABLE_SET__@@';
const SetPrototype = Set.prototype;
SetPrototype[IS_SET_SENTINEL] = true;
SetPrototype[DELETE] = SetPrototype.remove;
SetPrototype.mergeDeep = SetPrototype.merge;
SetPrototype.mergeDeepWith = SetPrototype.mergeWith;
SetPrototype.merge = SetPrototype.union;
SetPrototype.withMutations = MapPrototype.withMutations;
SetPrototype.asMutable = MapPrototype.asMutable;
SetPrototype.asImmutable = MapPrototype.asImmutable;
Expand Down
38 changes: 5 additions & 33 deletions type-definitions/Immutable.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,39 +675,6 @@ declare module Immutable {
update(index: number, updater: (value: T) => T): this;
update<R>(updater: (value: this) => R): R;

/**
* Note: `merge` can be used in `withMutations`.
*
* @see `Map#merge`
*/
merge(...collections: Array<Iterable<T>>): this;

/**
* Note: `mergeWith` can be used in `withMutations`.
*
* @see `Map#mergeWith`
*/
mergeWith(
merger: (oldVal: T, newVal: T, key: number) => T,
...collections: Array<Iterable<T>>
): this;

/**
* Note: `mergeDeep` can be used in `withMutations`.
*
* @see `Map#mergeDeep`
*/
mergeDeep(...collections: Array<Iterable<T>>): this;

/**
* Note: `mergeDeepWith` can be used in `withMutations`.
* @see `Map#mergeDeepWith`
*/
mergeDeepWith(
merger: (oldVal: T, newVal: T, key: number) => T,
...collections: Array<Iterable<T>>
): this;

/**
* Returns a new List with size `size`. If `size` is less than this
* List's size, the new List will exclude values at the higher indices.
Expand Down Expand Up @@ -819,8 +786,13 @@ declare module Immutable {

/**
* Returns a new List with other values or collections concatenated to this one.
*
* Note: `concat` *cannot* be safely used in `withMutations`.
*
* @alias merge
*/
concat<C>(...valuesOrCollections: Array<Iterable<C> | C>): List<T | C>;
merge<C>(...collections: Array<Iterable<C>): List<T | C>;

/**
* Returns a new List with values passed through a
Expand Down
12 changes: 0 additions & 12 deletions type-definitions/immutable.js.flow
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,18 +636,6 @@ declare class List<+T> extends IndexedCollection<T> {

merge<U>(...collections: Iterable<U>[]): List<T | U>;

mergeWith<U, V>(
merger: (oldVal: T, newVal: U, key: number) => V,
...collections: Iterable<U>[]
): List<T | U | V>;

mergeDeep<U>(...collections: Iterable<U>[]): List<T | U>;

mergeDeepWith<U, V>(
merger: (oldVal: T, newVal: U, key: number) => V,
...collections: Iterable<U>[]
): List<T | U | V>;

setSize(size: number): this;
setIn(keyPath: Iterable<mixed>, value: mixed): this;
deleteIn(keyPath: Iterable<mixed>, value: mixed): this;
Expand Down
12 changes: 0 additions & 12 deletions type-definitions/tests/immutable-flow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,6 @@ numberOrStringList = List.of('a').merge(List.of(1))
// $ExpectError
numberList = List.of('a').merge(List.of(1))

numberList = List.of(1).mergeWith((previous, next, key) => 1, [1])
// $ExpectError
numberList = List.of(1).mergeWith((previous, next, key) => previous + next, ['a'])

numberOrStringList = List.of(1).mergeDeep(['a'])
// $ExpectError
numberList = List.of(1).mergeDeep(['a'])

numberList = List.of(1).mergeDeepWith((previous, next, key) => 1, [1])
// $ExpectError
numberList = List.of(1).mergeDeepWith((previous, next, key) => previous + next, ['a'])

nullableNumberList = List.of(1).setSize(2)

numberList = List.of(1).setIn([], 0)
Expand Down
65 changes: 1 addition & 64 deletions type-definitions/ts-tests/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ import { List } from '../../';
// $ExpectType List<number>
List<number>().merge(List<number>());

// $ExpectError
// $ExpectType List<string | number>
List<number>().merge(List<string>());

// $ExpectType List<string | number>
Expand All @@ -304,75 +304,12 @@ import { List } from '../../';
List<number>().mergeIn([], []);
}

{ // #mergeWith

// $ExpectType List<number>
List<number>().mergeWith((prev: number, next: number, key: number) => 1, List<number>());

// $ExpectError
List<number>().mergeWith((prev: string, next: number, key: number) => 1, List<number>());

// $ExpectError
List<number>().mergeWith((prev: number, next: string, key: number) => 1, List<number>());

// $ExpectError
List<number>().mergeWith((prev: number, next: number, key: string) => 1, List<number>());

// $ExpectError
List<number>().mergeWith((prev: number, next: number, key: number) => 'a', List<number>());

// $ExpectError
List<number>().mergeWith((prev: number, next: number, key: number) => 1, List<string>());

// $ExpectType List<string | number>
List<number | string>().mergeWith((prev: number, next: string, key: number) => 1, List<string>());
}

{ // #mergeDeep

// $ExpectType List<number>
List<number>().mergeDeep(List<number>());

// $ExpectError
List<number>().mergeDeep(List<string>());

// $ExpectType List<string | number>
List<number | string>().mergeDeep(List<string>());

// $ExpectType List<string | number>
List<number | string>().mergeDeep(List<number>());
}

{ // #mergeDeepIn

// $ExpectType List<number>
List<number>().mergeDeepIn([], []);
}

{ // #mergeDeepWith

// $ExpectType List<number>
List<number>().mergeDeepWith((prev: number, next: number, key: number) => 1, List<number>());

// $ExpectError
List<number>().mergeDeepWith((prev: string, next: number, key: number) => 1, List<number>());

// $ExpectError
List<number>().mergeDeepWith((prev: number, next: string, key: number) => 1, List<number>());

// $ExpectError
List<number>().mergeDeepWith((prev: number, next: number, key: string) => 1, List<number>());

// $ExpectError
List<number>().mergeDeepWith((prev: number, next: number, key: number) => 'a', List<number>());

// $ExpectError
List<number>().mergeDeepWith((prev: number, next: number, key: number) => 1, List<string>());

// $ExpectType List<string | number>
List<number | string>().mergeDeepWith((prev: number, next: string, key: number) => 1, List<string>());
}

{ // #flatten

// $ExpectType Collection<any, any>
Expand Down
0