Support Symbol named observable properties

* Add tests for using symbol keys and getting them from toJS

src/api/computed.ts

@@ -20,7 +20,7 @@ export const computedDecorator = createPropDecorator(
     false,
     (
         instance: any,
-        propertyName: string,
+        propertyName: PropertyKey,
         descriptor: any,
         decoratorTarget: any,
         decoratorArgs: any[]

src/api/extendobservable.ts

@@ -14,7 +14,9 @@ import {
     isComputed,
     computedDecorator,
     endBatch,
-    fail
+    fail,
+    getPlainObjectKeys,
+    stringifyKey
 } from "../internal"
 
 export function extendShallowObservable<A extends Object, B extends Object>(
@@ -51,10 +53,16 @@ export function extendObservable<A extends Object, B extends Object>(
             !isObservable(properties),
             "Extending an object with another observable (object) is not supported. Please construct an explicit propertymap, using `toJS` if need. See issue #540"
         )
-        if (decorators)
-            for (let key in decorators)
-                if (!(key in properties))
-                    fail(`Trying to declare a decorator for unspecified property '${key}'`)
+        if (decorators) {
+            getPlainObjectKeys(decorators).forEach(key => {
+                if (!(key in properties!))
+                    fail(
+                        `Trying to declare a decorator for unspecified property '${stringifyKey(
+                            key
+                        )}'`
+                    )
+            })
+        }
     }
 
     options = asCreateObservableOptions(options)
@@ -64,12 +72,16 @@ export function extendObservable<A extends Object, B extends Object>(
     asObservableObject(target, options.name, defaultDecorator.enhancer) // make sure object is observable, even without initial props
     startBatch()
     try {
-        for (let key in properties) {
+        const keys = getPlainObjectKeys(properties)
+        for (let i in keys) {
+            const key = keys[i]
             const descriptor = Object.getOwnPropertyDescriptor(properties, key)!
             if (process.env.NODE_ENV !== "production") {
                 if (Object.getOwnPropertyDescriptor(target, key))
                     fail(
-                        `'extendObservable' can only be used to introduce new properties. Use 'set' or 'decorate' instead. The property '${key}' already exists on '${target}'`
+                        `'extendObservable' can only be used to introduce new properties. Use 'set' or 'decorate' instead. The property '${stringifyKey(
+                            key
+                        )}' already exists on '${target}'`
                     )
                 if (isComputed(descriptor.value))
                     fail(
@@ -83,7 +95,7 @@ export function extendObservable<A extends Object, B extends Object>(
                     ? computedDecorator
                     : defaultDecorator
             if (process.env.NODE_ENV !== "production" && typeof decorator !== "function")
-                return fail(`Not a valid decorator for '${key}', got: ${decorator}`)
+                return fail(`Not a valid decorator for '${stringifyKey(key)}', got: ${decorator}`)
 
             const resultDescriptor = decorator!(target, key, descriptor, true)
             if (

src/api/object-api.ts

@@ -20,7 +20,7 @@ import {
 export function keys<K>(map: ObservableMap<K, any>): ReadonlyArray<K>
 export function keys<T>(ar: IObservableArray<T>): ReadonlyArray<number>
 export function keys<T>(set: ObservableSet<T>): ReadonlyArray<T>
-export function keys<T extends Object>(obj: T): ReadonlyArray<string>
+export function keys<T extends Object>(obj: T): ReadonlyArray<PropertyKey>
 export function keys(obj: any): any {
     if (isObservableObject(obj)) {
         return ((obj as any) as IIsObservableObject).$mobx.getKeys()
@@ -86,12 +86,12 @@ export function entries(obj: any): any {
     )
 }
 
-export function set<V>(obj: ObservableMap<string, V>, values: { [key: string]: V })
+export function set<V>(obj: ObservableMap<PropertyKey, V>, values: { [key: string]: V })
 export function set<K, V>(obj: ObservableMap<K, V>, key: K, value: V)
 export function set<T>(obj: ObservableSet<T>, value: T)
 export function set<T>(obj: IObservableArray<T>, index: number, value: T)
 export function set<T extends Object>(obj: T, values: { [key: string]: any })
-export function set<T extends Object>(obj: T, key: string, value: any)
+export function set<T extends Object>(obj: T, key: PropertyKey, value: any)
 export function set(obj: any, key: any, value?: any): void {
     if (arguments.length === 2 && !isObservableSet(obj)) {
         startBatch()

src/api/observabledecorator.ts

@@ -4,7 +4,8 @@ import {
     IEnhancer,
     createPropDecorator,
     BabelDescriptor,
-    defineObservableProperty
+    defineObservableProperty,
+    stringifyKey
 } from "../internal"
 
 export type IObservableDecorator = {
@@ -17,15 +18,17 @@ export function createDecoratorForEnhancer(enhancer: IEnhancer<any>): IObservabl
         true,
         (
             target: any,
-            propertyName: string,
+            propertyName: PropertyKey,
             descriptor: BabelDescriptor,
             _decoratorTarget,
             decoratorArgs: any[]
         ) => {
             if (process.env.NODE_ENV !== "production") {
                 invariant(
                     !descriptor || !descriptor.get,
-                    `@observable cannot be used on getter (property "${propertyName}"), use @computed instead.`
+                    `@observable cannot be used on getter (property "${stringifyKey(
+                        propertyName
+                    )}"), use @computed instead.`
                 )
             }
             const initialValue = descriptor

src/api/tojs.ts

@@ -4,7 +4,8 @@ import {
     isObservableValue,
     keys,
     isObservableSet,
-    isObservableMap
+    isObservableMap,
+    getPlainObjectKeys
 } from "../internal"
 
 export type ToJSOptions = {
@@ -88,9 +89,9 @@ function toJSHelper(source, options: ToJSOptions, __alreadySeen: Map<any, any>)
 
     // Fallback to the situation that source is an ObservableObject or a plain object
     const res = cache(__alreadySeen, source, {}, options)
-    for (let key in source) {
+    getPlainObjectKeys(source).forEach(key => {
         res[key] = toJSHelper(source[key], options!, __alreadySeen)
-    }
+    })
 
     return res
 }

src/types/observablemap.ts

@@ -21,6 +21,7 @@ import {
     isSpyEnabled,
     hasListeners,
     spyReportStart,
+    stringifyKey,
     transaction,
     notifyListeners,
     spyReportEnd,
@@ -32,7 +33,8 @@ import {
     registerListener,
     IInterceptor,
     registerInterceptor,
-    declareIterator
+    declareIterator,
+    getPlainObjectKeys
 } from "../internal"
 
 export interface IKeyValueMap<V = any> {
@@ -299,7 +301,7 @@ export class ObservableMap<K = any, V = any>
         }
         transaction(() => {
             if (isPlainObject(other))
-                Object.keys(other).forEach(key => this.set((key as any) as K, other[key]))
+                getPlainObjectKeys(other).forEach(key => this.set((key as any) as K, other[key]))
             else if (Array.isArray(other)) other.forEach(([key, value]) => this.set(key, value))
             else if (isES6Map(other)) {
                 if (other.constructor !== Map)
@@ -394,11 +396,6 @@ 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()
-}
-
 declareIterator(ObservableMap.prototype, function() {
     return this.entries()
 })

src/types/observableobject.ts

@@ -29,29 +29,31 @@ import {
     IComputedValueOptions,
     initializeInstance,
     createInstanceofPredicate,
-    isObject
+    isObject,
+    stringifyKey
 } from "../internal"
+import { getPlainObjectKeys } from "../utils/utils"
 
 export interface IObservableObject {
     "observable-object": IObservableObject
 }
 
 export type IObjectDidChange =
     | {
-          name: string
+          name: PropertyKey
           object: any
           type: "add"
           newValue: any
       }
     | {
-          name: string
+          name: PropertyKey
           object: any
           type: "update"
           oldValue: any
           newValue: any
       }
     | {
-          name: string
+          name: PropertyKey
           object: any
           type: "remove"
           oldValue: any
@@ -61,33 +63,34 @@ export type IObjectWillChange =
     | {
           object: any
           type: "update" | "add"
-          name: string
+          name: PropertyKey
           newValue: any
       }
     | {
           object: any
           type: "remove"
-          name: string
+          name: PropertyKey
       }
 
 export class ObservableObjectAdministration
     implements IInterceptable<IObjectWillChange>, IListenable {
-    values: { [key: string]: ObservableValue<any> | ComputedValue<any> } = {}
-    keys: undefined | IObservableArray<string>
+    // @ts-ignore
+    values: { [key: PropertyKey]: ObservableValue<any> | ComputedValue<any> } = {}
+    keys: undefined | IObservableArray<PropertyKey>
     changeListeners
     interceptors
 
     constructor(public target: any, public name: string, public defaultEnhancer: IEnhancer<any>) {}
 
-    read(owner: any, key: string) {
+    read(owner: any, key: PropertyKey) {
         if (process.env.NODE_ENV === "production" && this.target !== owner) {
             this.illegalAccess(owner, key)
             if (!this.values[key]) return undefined
         }
         return this.values[key].get()
     }
 
-    write(owner: any, key: string, newValue) {
+    write(owner: any, key: PropertyKey, newValue) {
         const instance = this.target
         if (process.env.NODE_ENV === "production" && instance !== owner) {
             this.illegalAccess(owner, key)
@@ -133,7 +136,7 @@ export class ObservableObjectAdministration
         }
     }
 
-    remove(key: string) {
+    remove(key: PropertyKey) {
         if (!this.values[key]) return
         const { target } = this
         if (hasInterceptors(this)) {
@@ -169,7 +172,7 @@ export class ObservableObjectAdministration
         }
     }
 
-    illegalAccess(owner, propName) {
+    illegalAccess(owner, propName: PropertyKey) {
         /**
          * This happens if a property is accessed through the prototype chain, but the property was
          * declared directly as own property on the prototype.
@@ -190,7 +193,9 @@ export class ObservableObjectAdministration
          * When using decorate, the property will always be redeclared as own property on the actual instance
          */
         console.warn(
-            `Property '${propName}' of '${owner}' was accessed through the prototype chain. Use 'decorate' instead to declare the prop or access it statically through it's owner`
+            `Property '${stringifyKey(
+                propName
+            )}' of '${owner}' was accessed through the prototype chain. Use 'decorate' instead to declare the prop or access it statically through it's owner`
         )
     }
 
@@ -212,11 +217,11 @@ export class ObservableObjectAdministration
         return registerInterceptor(this, handler)
     }
 
-    getKeys(): string[] {
+    getKeys(): PropertyKey[] {
         if (this.keys === undefined) {
             this.keys = <any>(
                 new ObservableArray(
-                    Object.keys(this.values).filter(
+                    getPlainObjectKeys(this.values).filter(
                         key => this.values[key] instanceof ObservableValue
                     ),
                     referenceEnhancer,
@@ -257,7 +262,7 @@ export function asObservableObject(
 
 export function defineObservableProperty(
     target: any,
-    propName: string,
+    propName: PropertyKey,
     newValue,
     enhancer: IEnhancer<any>
 ) {
@@ -277,7 +282,7 @@ export function defineObservableProperty(
     const observable = (adm.values[propName] = new ObservableValue(
         newValue,
         enhancer,
-        `${adm.name}.${propName}`,
+        `${adm.name}.${stringifyKey(propName)}`,
         false
     ))
     newValue = (observable as any).value // observableValue might have changed it
@@ -289,11 +294,11 @@ export function defineObservableProperty(
 
 export function defineComputedProperty(
     target: any, // which objects holds the observable and provides `this` context?
-    propName: string,
+    propName: PropertyKey,
     options: IComputedValueOptions<any>
 ) {
     const adm = asObservableObject(target)
-    options.name = `${adm.name}.${propName}`
+    options.name = `${adm.name}.${stringifyKey(propName)}`
     options.context = target
     adm.values[propName] = new ComputedValue(options)
     Object.defineProperty(target, propName, generateComputedPropConfig(propName))
@@ -348,7 +353,7 @@ export function generateComputedPropConfig(propName) {
 function notifyPropertyAddition(
     adm: ObservableObjectAdministration,
     object,
-    key: string,
+    key: PropertyKey,
     newValue
 ) {
     const notify = hasListeners(adm)

src/utils/decorators2.ts

@@ -9,7 +9,7 @@ export type BabelDescriptor = PropertyDescriptor & { initializer?: () => any }
 
 export type PropertyCreator = (
     instance: any,
-    propertyName: string,
+    propertyName: PropertyKey,
     descriptor: BabelDescriptor | undefined,
     decoratorTarget: any,
     decoratorArgs: any[]