fix makeAutoObservable + symbolic keys

mobxjs · Feb 2, 2021 · 48664d0 · 48664d0
1 parent ddf9978
commit 48664d0
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 10 deletions.
diff --git a/.changeset/two-rings-lay.md b/.changeset/two-rings-lay.md
@@ -0,0 +1,6 @@
+---
+"mobx": patch
+---
+
+Fix: `makeAutoObservable` now working properly with symbolic keys
+Fix: `isComputedProp` and `getAtom` second arg type is incompatible with Symbols
diff --git a/packages/mobx/__tests__/v5/base/make-observable.ts b/packages/mobx/__tests__/v5/base/make-observable.ts
@@ -1449,3 +1449,29 @@ test("inherited fields are assignable before makeObservable", () => {
     expect(isAction(foo2.action)).toBe(true)
     expect(isFlow(foo2.flow)).toBe(true)
 })
+
+test("makeAutoObservable + symbolic keys", () => {
+    const observableSymbol = Symbol()
+    const computedSymbol = Symbol()
+    const actionSymbol = Symbol()
+
+    class Foo {
+        observable = "observable";
+        [observableSymbol] = "observableSymbol"
+        get [computedSymbol]() {
+            return this.observable
+        }
+        [actionSymbol]() {}
+
+        constructor() {
+            makeAutoObservable(this)
+        }
+    }
+
+    ;[new Foo(), new Foo()].forEach(foo => {
+        expect(isObservableProp(foo, "observable")).toBe(true)
+        expect(isObservableProp(foo, observableSymbol)).toBe(true)
+        expect(isComputedProp(foo, computedSymbol)).toBe(true)
+        expect(isAction(foo[actionSymbol])).toBe(true)
+    })
+})
diff --git a/packages/mobx/src/api/iscomputed.ts b/packages/mobx/src/api/iscomputed.ts
@@ -1,6 +1,6 @@
 import { $mobx, getAtom, isComputedValue, isObservableObject, die, isStringish } from "../internal"
 
-export function _isComputed(value, property?: string): boolean {
+export function _isComputed(value, property?: PropertyKey): boolean {
     if (property !== undefined) {
         if (isObservableObject(value) === false) return false
         if (!value[$mobx].values_.has(property)) return false
@@ -18,7 +18,7 @@ export function isComputed(value: any): boolean {
     return _isComputed(value)
 }
 
-export function isComputedProp(value: any, propName: string): boolean {
+export function isComputedProp(value: any, propName: PropertyKey): boolean {
     if (__DEV__ && !isStringish(propName))
         return die(`isComputed expected a property name as second argument`)
     return _isComputed(value, propName)

diff --git a/packages/mobx/src/api/makeObservable.ts b/packages/mobx/src/api/makeObservable.ts
@@ -63,10 +63,9 @@ export function makeAutoObservable<T extends object, AdditionalKeys extends Prop
     const adm: ObservableObjectAdministration = asObservableObject(target, options)[$mobx]
     startBatch()
     try {
+        // Use cached inferred annotations if available (only in classes)
         if (target[inferredAnnotationsSymbol]) {
-            for (let key in target[inferredAnnotationsSymbol]) {
-                adm.make_(key, target[inferredAnnotationsSymbol][key])
-            }
+            target[inferredAnnotationsSymbol].forEach((value, key) => adm.make_(key, value))
         } else {
             const ignoreKeys = { [$mobx]: 1, [inferredAnnotationsSymbol]: 1, constructor: 1 }
             const make = key => {

diff --git a/packages/mobx/src/types/observableobject.ts b/packages/mobx/src/types/observableobject.ts
@@ -47,7 +47,7 @@ import {
     objectPrototype
 } from "../internal"
 
-// adm[inferredAnnotationsSymbol] = { foo: annotation, ... }
+// closestPrototypeofTarget[inferredAnnotationsSymbol] = new Map<PropertyKes, Annotation>()
 export const inferredAnnotationsSymbol = Symbol("mobx-inferred-annotations")
 
 const descriptorCache = Object.create(null)
@@ -280,7 +280,7 @@ export class ObservableObjectAdministration
 
     inferAnnotation_(key: PropertyKey): Annotation | false {
         // Inherited is fine - annotation cannot differ in subclass
-        let annotation = this.target_[inferredAnnotationsSymbol]?.[key]
+        let annotation = this.target_[inferredAnnotationsSymbol]?.get(key)
         if (annotation) return annotation
 
         let current = this.target_
@@ -308,9 +308,9 @@ export class ObservableObjectAdministration
             // We could also place it on furthest proto, shoudn't matter
             const closestProto = Object.getPrototypeOf(this.target_)
             if (!hasProp(closestProto, inferredAnnotationsSymbol)) {
-                addHiddenProp(closestProto, inferredAnnotationsSymbol, {})
+                addHiddenProp(closestProto, inferredAnnotationsSymbol, new Map())
             }
-            closestProto[inferredAnnotationsSymbol][key] = annotation
+            closestProto[inferredAnnotationsSymbol].set(key, annotation)
         }
 
         return annotation

diff --git a/packages/mobx/src/types/type-utils.ts b/packages/mobx/src/types/type-utils.ts
@@ -12,7 +12,7 @@ import {
     isFunction
 } from "../internal"
 
-export function getAtom(thing: any, property?: string): IDepTreeNode {
+export function getAtom(thing: any, property?: PropertyKey): IDepTreeNode {
     if (typeof thing === "object" && thing !== null) {
         if (isObservableArray(thing)) {
             if (property !== undefined) die(23)