diff --git a/CHANGELOG.md b/CHANGELOG.md index 1015c0758..322750bcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # 5.13.0 / 4.13.0 +* Fixed bug that observable map replace function breaks entities order. related issues:[#1980](https://github.com/mobxjs/mobx/issues/1980) and [#2003](https://github.com/mobxjs/mobx/issues/2003) * Fixed potential memory leak in observable maps, when non-primitive values are used as keys. Fixes [#2031](https://github.com/mobxjs/mobx/issues/2031) through [#2032](https://github.com/mobxjs/mobx/pull/2032). * Added support to store additional non-observable(!) fields (string or symbol based) on array, to better reflect behavior of MobX 4. Fixes [#2044](https://github.com/mobxjs/mobx/issues/2044) through [#2046](https://github.com/mobxjs/mobx/pull/2046) diff --git a/src/types/observablemap.ts b/src/types/observablemap.ts index f4b558310..8ac5df527 100644 --- a/src/types/observablemap.ts +++ b/src/types/observablemap.ts @@ -11,7 +11,6 @@ import { createInstanceofPredicate, deepEnhancer, fail, - getMapLikeKeys, getNextId, getPlainObjectKeys, hasInterceptors, @@ -32,7 +31,8 @@ import { transaction, untracked, onBecomeUnobserved, - globalState + globalState, + convertToMap } from "../internal" export interface IKeyValueMap { @@ -335,11 +335,24 @@ export class ObservableMap // grab all the keys that are present in the new map but not present in the current map // and delete them from the map, then merge the new map // this will cause reactions only on changed values - const newKeys = (getMapLikeKeys(values) as any) as K[] + const replacementMap = convertToMap(values) + const orderedData = new Map() const oldKeys = Array.from(this.keys()) - const missingKeys = oldKeys.filter(k => newKeys.indexOf(k) === -1) - missingKeys.forEach(k => this.delete(k)) - this.merge(values) + + for (let i = 0; i < oldKeys.length; i++) { + const oldKey = oldKeys[i] + // delete missing key + if (!replacementMap.has(oldKey)) { + this.delete(oldKey) + } + } + // set new keys in the right order + replacementMap.forEach((value, key) => { + this.set(key, value) + orderedData.set(key, this._data.get(key)) + }) + // use data with correct order + this._data = orderedData }) return this } diff --git a/src/utils/utils.ts b/src/utils/utils.ts index ca9aaa18d..7998ca39f 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -88,6 +88,18 @@ export function isPlainObject(value) { return proto === Object.prototype || proto === null } +export function convertToMap(dataStructure) { + if (isES6Map(dataStructure) || isObservableMap(dataStructure)) { + return dataStructure + } else if (Array.isArray(dataStructure)) { + return new Map(dataStructure) + } else if (isPlainObject(dataStructure)) { + return new Map(Object.entries(dataStructure)) + } else { + return fail(`Cannot convert to map from '${dataStructure}'`) + } +} + export function makeNonEnumerable(object: any, propNames: PropertyKey[]) { for (let i = 0; i < propNames.length; i++) { addHiddenProp(object, propNames[i], object[propNames[i]]) diff --git a/test/base/map.js b/test/base/map.js index 45e653055..9439f5ca2 100644 --- a/test/base/map.js +++ b/test/base/map.js @@ -618,11 +618,22 @@ test("issue 1243, .replace should not trigger change on unchanged values", () => expect(() => { m.replace("not-an-object") - }).toThrow(/Cannot get keys from 'not-an-object'/) + }).toThrow(/Cannot convert to map from 'not-an-object'/) d() }) +test("#1980 .replace should not breaks entities order!", () => { + const original = mobx.observable.map([["a", "first"], ["b", "second"]]) + const replacement = new Map([["b", "first"], ["a", "second"]]) + original.replace(replacement) + const newKeys = Array.from(replacement) + const originalKeys = Array.from(replacement) + for (let i = 0; i < newKeys.length; i++) { + expect(newKeys[i]).toEqual(originalKeys[i]) + } +}) + test("#1258 cannot replace maps anymore", () => { const items = mobx.observable.map() items.replace(mobx.observable.map())