fix issues 1980 and 2003 (#2057)

* fix issues 1980 and 2003

* Update CHANGELOG.md

Add changelog for fixes 1980 and 2003 issues.

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)
 

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<V = any> {
@@ -335,11 +335,24 @@ export class ObservableMap<K = any, V = any>
             // 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
     }

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]])

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())