Allow Symbol keys in observable maps (fixes #1925)

mobxjs · Mar 14, 2019 · 9f12565 · 9f12565
1 parent 4451340
commit 9f12565
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# Unreleased
+
+* Allow symbol keys in `ObservableMap`, see [#1930](https://github.com/mobxjs/mobx/pull/1930) by [pimterry](https://github.com/pimterry)
+
 # 4.9.3
 
 * Fixed `observable.set` compatibility with IE 11, see [#1917](https://github.com/mobxjs/mobx/pull/1917) by [kalmi](https://github.com/kalmi)

diff --git a/src/types/observablemap.ts b/src/types/observablemap.ts
@@ -174,7 +174,12 @@ export class ObservableMap<K = any, V = any>
         if (entry) {
             entry.setNewValue(value)
         } else {
-            entry = new ObservableValue(value, referenceEnhancer, `${this.name}.${key}?`, false)
+            entry = new ObservableValue(
+                value,
+                referenceEnhancer,
+                `${this.name}.${stringifyKey(key)}?`,
+                false
+            )
             this._hasMap.set(key, entry)
         }
         return entry
@@ -210,7 +215,7 @@ export class ObservableMap<K = any, V = any>
             const observable = new ObservableValue(
                 newValue,
                 this.enhancer,
-                `${this.name}.${key}`,
+                `${this.name}.${stringifyKey(key)}`,
                 false
             )
             this._data.set(key, observable)
@@ -345,7 +350,8 @@ export class ObservableMap<K = any, V = any>
     toPOJO(): IKeyValueMap<V> {
         const res: IKeyValueMap<V> = {}
         for (const [key, value] of this) {
-            res["" + key] = value
+            // We lie about symbol key types due to https://github.com/Microsoft/TypeScript/issues/1863
+            res[typeof key === "symbol" ? <any>key : stringifyKey(key)] = value
         }
         return res
     }
@@ -368,7 +374,7 @@ export class ObservableMap<K = any, V = any>
             this.name +
             "[{ " +
             Array.from(this.keys())
-                .map(key => `${key}: ${"" + this.get(key)}`)
+                .map(key => `${stringifyKey(key)}: ${"" + this.get(key)}`)
                 .join(", ") +
             " }]"
         )
@@ -395,6 +401,11 @@ export class ObservableMap<K = any, V = any>
     }
 }
 
+function stringifyKey(key: any): string {
+    if (key && key.toString) return key.toString()
+    else return new String(key).toString()
+}
+
 /* 'var' fixes small-build issue */
 export const isObservableMap = createInstanceofPredicate("ObservableMap", ObservableMap) as (
     thing: any

diff --git a/test/base/map.js b/test/base/map.js
@@ -31,14 +31,23 @@ test("map crud", function() {
     expect(m.has(k)).toBe(true)
     expect(m.get(k)).toBe("arrVal")
 
-    expect(mobx.keys(m)).toEqual(["1", 1, k])
-    expect(mobx.values(m)).toEqual(["aa", "b", "arrVal"])
-    expect(Array.from(m)).toEqual([["1", "aa"], [1, "b"], [k, "arrVal"]])
-    expect(m.toJS()).toEqual(new Map([["1", "aa"], [1, "b"], [k, "arrVal"]]))
-    expect(m.toPOJO()).toEqual({ "1": "b", arr: "arrVal" })
+    var s = Symbol("test")
+    expect(m.has(s)).toBe(false)
+    expect(m.get(s)).toBe(undefined)
+    m.set(s, "symbol-value")
+    expect(m.get(s)).toBe("symbol-value")
+    expect(m.get(s.toString())).toBe(undefined)
+
+    expect(mobx.keys(m)).toEqual(["1", 1, k, s])
+    expect(mobx.values(m)).toEqual(["aa", "b", "arrVal", "symbol-value"])
+    expect(Array.from(m)).toEqual([["1", "aa"], [1, "b"], [k, "arrVal"], [s, "symbol-value"]])
+    expect(m.toJS()).toEqual(new Map([["1", "aa"], [1, "b"], [k, "arrVal"], [s, "symbol-value"]]))
+    expect(m.toPOJO()).toEqual({ "1": "b", arr: "arrVal", [s]: "symbol-value" })
     expect(JSON.stringify(m)).toEqual('{"1":"b","arr":"arrVal"}')
-    expect(m.toString()).toBe("ObservableMap@1[{ 1: aa, 1: b, arr: arrVal }]")
-    expect(m.size).toBe(3)
+    expect(m.toString()).toBe(
+        "ObservableMap@1[{ 1: aa, 1: b, arr: arrVal, Symbol(test): symbol-value }]"
+    )
+    expect(m.size).toBe(4)
 
     m.clear()
     expect(mobx.keys(m)).toEqual([])
@@ -56,9 +65,11 @@ test("map crud", function() {
         { object: m, name: "1", newValue: "aa", oldValue: "a", type: "update" },
         { object: m, name: 1, newValue: "b", type: "add" },
         { object: m, name: ["arr"], newValue: "arrVal", type: "add" },
+        { object: m, name: s, newValue: "symbol-value", type: "add" },
         { object: m, name: "1", oldValue: "aa", type: "delete" },
         { object: m, name: 1, oldValue: "b", type: "delete" },
-        { object: m, name: ["arr"], oldValue: "arrVal", type: "delete" }
+        { object: m, name: ["arr"], oldValue: "arrVal", type: "delete" },
+        { object: m, name: s, oldValue: "symbol-value", type: "delete" }
     ])
 })