test: better collection coverage + tests for immutable

packages/observer/src/autorun.ts

@@ -116,7 +116,9 @@ export function trigger(
     })
   } else {
     // schedule runs for SET | ADD | DELETE
-    addRunners(runners, depsMap.get(key as string | symbol))
+    if (key !== void 0) {
+      addRunners(runners, depsMap.get(key as string | symbol))
+    }
     // also run for iteration key on ADD | DELETE
     if (type === OperationTypes.ADD || type === OperationTypes.DELETE) {
       const iterationKey = Array.isArray(target) ? 'length' : ITERATE_KEY

packages/observer/src/baseHandlers.ts

@@ -15,14 +15,14 @@ function get(
   target: any,
   key: string | symbol,
   receiver: any,
-  toObsevable: (t: any) => any
+  toObservable: (t: any) => any
 ) {
   const res = Reflect.get(target, key, receiver)
   if (typeof key === 'symbol' && builtInSymbols.has(key)) {
     return res
   }
   track(target, OperationTypes.GET, key)
-  return res !== null && typeof res === 'object' ? toObsevable(res) : res
+  return res !== null && typeof res === 'object' ? toObservable(res) : res
 }
 
 function set(
@@ -96,7 +96,10 @@ export const immutableHandlers: ProxyHandler<any> = {
   set(target: any, key: string | symbol, value: any, receiver: any): boolean {
     if (LOCKED) {
       if (__DEV__) {
-        console.warn(`Set operation failed: target is immutable.`, target)
+        console.warn(
+          `Set operation on key "${key as any}" failed: target is immutable.`,
+          target
+        )
       }
       return true
     } else {
@@ -107,7 +110,10 @@ export const immutableHandlers: ProxyHandler<any> = {
   deleteProperty(target: any, key: string | symbol): boolean {
     if (LOCKED) {
       if (__DEV__) {
-        console.warn(`Delete operation failed: target is immutable.`, target)
+        console.warn(
+          `Delete operation on key "${key as any}" failed: target is immutable.`,
+          target
+        )
       }
       return true
     } else {

packages/observer/src/collectionHandlers.ts

@@ -1,18 +1,24 @@
-import { unwrap } from './index'
+import { unwrap, observable, immutable } from './index'
 import { track, trigger } from './autorun'
 import { OperationTypes } from './operations'
+import { LOCKED } from './lock'
 
-function makeInstrumentedMethod(method: string | symbol, type: OperationTypes) {
-  return function(...args: any[]) {
-    const target = unwrap(this)
-    const proto: any = Reflect.getPrototypeOf(target)
-    track(target, type, args[0])
-    return proto[method].apply(target, args)
-  }
+function get(target: any, key: any, toObservable: (t: any) => any): any {
+  target = unwrap(target)
+  key = unwrap(key)
+  const proto: any = Reflect.getPrototypeOf(target)
+  track(target, OperationTypes.GET, key)
+  const res = proto.get.call(target, key)
+  return res !== null && typeof res === 'object' ? toObservable(res) : res
 }
 
-const get = makeInstrumentedMethod('get', OperationTypes.GET)
-const has = makeInstrumentedMethod('has', OperationTypes.HAS)
+function has(key: any): boolean {
+  const target = unwrap(this)
+  key = unwrap(key)
+  const proto: any = Reflect.getPrototypeOf(target)
+  track(target, OperationTypes.HAS, key)
+  return proto.has.call(target, key)
+}
 
 function size(target: any) {
   target = unwrap(target)
@@ -21,115 +27,135 @@ function size(target: any) {
   return Reflect.get(proto, 'size', target)
 }
 
-function makeWarning(type: OperationTypes) {
-  return function() {
+function add(value: any) {
+  value = unwrap(value)
+  const target = unwrap(this)
+  const proto: any = Reflect.getPrototypeOf(this)
+  const hadKey = proto.has.call(target, value)
+  const result = proto.add.call(target, value)
+  if (!hadKey) {
     if (__DEV__) {
-      console.warn(
-        `${type} operation failed: target is immutable.`,
-        unwrap(this)
-      )
+      trigger(target, OperationTypes.ADD, value, { value })
+    } else {
+      trigger(target, OperationTypes.ADD, value)
+    }
+  }
+  return result
+}
+
+function set(key: any, value: any) {
+  value = unwrap(value)
+  const target = unwrap(this)
+  const proto: any = Reflect.getPrototypeOf(this)
+  const hadKey = proto.has.call(target, key)
+  const oldValue = proto.get.call(target, key)
+  const result = proto.set.call(target, key, value)
+  if (value !== oldValue) {
+    if (__DEV__) {
+      const extraInfo = { oldValue, newValue: value }
+      if (!hadKey) {
+        trigger(target, OperationTypes.ADD, key, extraInfo)
+      } else {
+        trigger(target, OperationTypes.SET, key, extraInfo)
+      }
+    } else {
+      if (!hadKey) {
+        trigger(target, OperationTypes.ADD, key)
+      } else {
+        trigger(target, OperationTypes.SET, key)
+      }
+    }
+  }
+  return result
+}
+
+function deleteEntry(key: any) {
+  const target = unwrap(this)
+  const proto: any = Reflect.getPrototypeOf(this)
+  const hadKey = proto.has.call(target, key)
+  const oldValue = proto.get ? proto.get.call(target, key) : undefined
+  // forward the operation before queueing reactions
+  const result = proto.delete.call(target, key)
+  if (hadKey) {
+    if (__DEV__) {
+      trigger(target, OperationTypes.DELETE, key, { oldValue })
+    } else {
+      trigger(target, OperationTypes.DELETE, key)
+    }
+  }
+  return result
+}
+
+function clear() {
+  const target = unwrap(this)
+  const proto: any = Reflect.getPrototypeOf(this)
+  const hadItems = target.size !== 0
+  const oldTarget = target instanceof Map ? new Map(target) : new Set(target)
+  // forward the operation before queueing reactions
+  const result = proto.clear.call(target)
+  if (hadItems) {
+    if (__DEV__) {
+      trigger(target, OperationTypes.CLEAR, void 0, { oldTarget })
+    } else {
+      trigger(target, OperationTypes.CLEAR)
+    }
+  }
+  return result
+}
+
+function makeImmutableMethod(method: Function, type: OperationTypes): Function {
+  return function(...args: any[]) {
+    if (LOCKED) {
+      if (__DEV__) {
+        const key = args[0] ? `on key "${args[0]}"` : ``
+        console.warn(
+          `${type} operation ${key}failed: target is immutable.`,
+          unwrap(this)
+        )
+      }
+      return type === OperationTypes.DELETE ? false : this
+    } else {
+      return method.apply(this, args)
     }
   }
 }
 
 const mutableInstrumentations: any = {
-  get,
-  has,
-
+  get(key: any) {
+    return get(this, key, observable)
+  },
   get size() {
     return size(this)
   },
-
-  add(key: any) {
-    const target = unwrap(this)
-    const proto: any = Reflect.getPrototypeOf(this)
-    const hadKey = proto.has.call(target, key)
-    const result = proto.add.apply(target, arguments)
-    if (!hadKey) {
-      if (__DEV__) {
-        trigger(target, OperationTypes.ADD, key, { value: key })
-      } else {
-        trigger(target, OperationTypes.ADD, key)
-      }
-    }
-    return result
-  },
-
-  set(key: any, value: any) {
-    const target = unwrap(this)
-    const proto: any = Reflect.getPrototypeOf(this)
-    const hadKey = proto.has.call(target, key)
-    const oldValue = proto.get.call(target, key)
-    const result = proto.set.apply(target, arguments)
-    if (value !== oldValue) {
-      if (__DEV__) {
-        const extraInfo = { oldValue, newValue: value }
-        if (!hadKey) {
-          trigger(target, OperationTypes.ADD, key, extraInfo)
-        } else {
-          trigger(target, OperationTypes.SET, key, extraInfo)
-        }
-      } else {
-        if (!hadKey) {
-          trigger(target, OperationTypes.ADD, key)
-        } else {
-          trigger(target, OperationTypes.SET, key)
-        }
-      }
-    }
-    return result
-  },
-
-  delete(key: any) {
-    const target = unwrap(this)
-    const proto: any = Reflect.getPrototypeOf(this)
-    const hadKey = proto.has.call(target, key)
-    const oldValue = proto.get ? proto.get.call(target, key) : undefined
-    // forward the operation before queueing reactions
-    const result = proto.delete.apply(target, arguments)
-    if (hadKey) {
-      if (__DEV__) {
-        trigger(target, OperationTypes.DELETE, key, { oldValue })
-      } else {
-        trigger(target, OperationTypes.DELETE, key)
-      }
-    }
-    return result
-  },
-
-  clear() {
-    const target = unwrap(this)
-    const proto: any = Reflect.getPrototypeOf(this)
-    const hadItems = target.size !== 0
-    const oldTarget = target instanceof Map ? new Map(target) : new Set(target)
-    // forward the operation before queueing reactions
-    const result = proto.clear.apply(target, arguments)
-    if (hadItems) {
-      if (__DEV__) {
-        trigger(target, OperationTypes.CLEAR, void 0, { oldTarget })
-      } else {
-        trigger(target, OperationTypes.CLEAR)
-      }
-    }
-    return result
-  }
+  has,
+  add,
+  set,
+  delete: deleteEntry,
+  clear
 }
 
 const immutableInstrumentations: any = {
-  get,
-  has,
+  get(key: any) {
+    return get(this, key, immutable)
+  },
   get size() {
     return size(this)
   },
-  add: makeWarning(OperationTypes.ADD),
-  set: makeWarning(OperationTypes.SET),
-  delete: makeWarning(OperationTypes.DELETE),
-  clear: makeWarning(OperationTypes.CLEAR)
+  has,
+  add: makeImmutableMethod(add, OperationTypes.ADD),
+  set: makeImmutableMethod(set, OperationTypes.SET),
+  delete: makeImmutableMethod(deleteEntry, OperationTypes.DELETE),
+  clear: makeImmutableMethod(clear, OperationTypes.CLEAR)
 }
-;['forEach', 'keys', 'values', 'entries', Symbol.iterator].forEach(key => {
-  mutableInstrumentations[key] = immutableInstrumentations[
-    key
-  ] = makeInstrumentedMethod(key, OperationTypes.ITERATE)
+;['forEach', 'keys', 'values', 'entries', Symbol.iterator].forEach(method => {
+  mutableInstrumentations[method] = immutableInstrumentations[
+    method
+  ] = function(...args: any[]) {
+    const target = unwrap(this)
+    const proto: any = Reflect.getPrototypeOf(target)
+    track(target, OperationTypes.ITERATE)
+    return proto[method].apply(target, args)
+  }
 })
 
 function getInstrumented(

packages/observer/src/index.ts

@@ -105,7 +105,9 @@ function createObservable(
   observed = new Proxy(target, handlers)
   toProxy.set(target, observed)
   toRaw.set(observed, target)
-  targetMap.set(target, new Map())
+  if (!targetMap.has(target)) {
+    targetMap.set(target, new Map())
+  }
   return observed
 }