test: 100% coverage for observer

.gitignore

@@ -1,5 +1,6 @@
 dist
 .DS_Store
 node_modules
+coverage
 explorations
 TODOs.md

package.json

@@ -6,7 +6,8 @@
   "scripts": {
     "dev": "node scripts/dev.js",
     "build": "node scripts/build.js",
-    "lint": "prettier --write --parser typescript 'packages/**/*.ts'"
+    "lint": "prettier --write --parser typescript 'packages/**/*.ts'",
+    "test": "jest"
   },
   "gitHooks": {
     "pre-commit": "lint-staged",

packages/observer/__tests__/autorun.spec.ts

@@ -621,6 +621,10 @@ describe('observer/autorun', () => {
     stop(runner)
     obj.prop = 3
     expect(dummy).toBe(2)
+
+    // stopped runner should still be manually callable
+    runner()
+    expect(dummy).toBe(3)
   })
 
   it('markNonReactive', () => {

packages/observer/__tests__/immutable.spec.ts

@@ -52,16 +52,26 @@ describe('observer/immutable', () => {
       observed.bar.baz = 3
       expect(observed.bar.baz).toBe(2)
       expect(warn).toHaveBeenCalledTimes(2)
+      delete observed.foo
+      expect(observed.foo).toBe(1)
+      expect(warn).toHaveBeenCalledTimes(3)
+      delete observed.bar.baz
+      expect(observed.bar.baz).toBe(2)
+      expect(warn).toHaveBeenCalledTimes(4)
     })
 
     it('should allow mutation when unlocked', () => {
-      const observed = immutable({ foo: 1, bar: { baz: 2 } })
+      const observed: any = immutable({ foo: 1, bar: { baz: 2 } })
       unlock()
-      observed.foo = 2
-      observed.bar.baz = 3
+      observed.prop = 2
+      observed.bar.qux = 3
+      delete observed.bar.baz
+      delete observed.foo
       lock()
-      expect(observed.foo).toBe(2)
-      expect(observed.bar.baz).toBe(3)
+      expect(observed.prop).toBe(2)
+      expect(observed.foo).toBeUndefined()
+      expect(observed.bar.qux).toBe(3)
+      expect('baz' in observed.bar).toBe(false)
       expect(warn).not.toHaveBeenCalled()
     })
 
@@ -190,7 +200,9 @@ describe('observer/immutable', () => {
       lock()
     })
   })
-  ;[Map, WeakMap].forEach((Collection: any) => {
+
+  const maps = [Map, WeakMap]
+  maps.forEach((Collection: any) => {
     describe(Collection.name, () => {
       test('should make nested values immutable', () => {
         const key1 = {}
@@ -224,22 +236,25 @@ describe('observer/immutable', () => {
 
       test('should allow mutation & trigger autorun when unlocked', () => {
         const map = immutable(new Collection())
+        const isWeak = Collection === WeakMap
         const key = {}
         let dummy
         autorun(() => {
-          dummy = map.get(key)
+          dummy = map.get(key) + (isWeak ? 0 : map.size)
         })
-        expect(dummy).toBeUndefined()
+        expect(dummy).toBeNaN()
         unlock()
         map.set(key, 1)
         lock()
-        expect(dummy).toBe(1)
+        expect(dummy).toBe(isWeak ? 1 : 2)
         expect(map.get(key)).toBe(1)
         expect(warn).not.toHaveBeenCalled()
       })
     })
   })
-  ;[Set, WeakSet].forEach((Collection: any) => {
+
+  const sets = [Set, WeakSet]
+  sets.forEach((Collection: any) => {
     describe(Collection.name, () => {
       test('should make nested values immutable', () => {
         const key1 = {}

packages/observer/src/baseHandlers.ts

@@ -11,18 +11,21 @@ const builtInSymbols = new Set(
     .filter(value => typeof value === 'symbol')
 )
 
-function get(
-  target: any,
-  key: string | symbol,
-  receiver: any,
-  toObservable: (t: any) => any
-) {
-  const res = Reflect.get(target, key, receiver)
-  if (typeof key === 'symbol' && builtInSymbols.has(key)) {
-    return res
+function makeGetter(isImmutable: boolean) {
+  return function get(target: any, key: string | symbol, receiver: 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'
+      ? isImmutable
+        ? // need to lazy access immutable and observable here to avoid
+          // circular dependency
+          immutable(res)
+        : observable(res)
+      : res
   }
-  track(target, OperationTypes.GET, key)
-  return res !== null && typeof res === 'object' ? toObservable(res) : res
 }
 
 function set(
@@ -37,6 +40,7 @@ function set(
   const result = Reflect.set(target, key, value, receiver)
   // don't trigger if target is something up in the prototype chain of original
   if (target === unwrap(receiver)) {
+    /* istanbul ignore else */
     if (__DEV__) {
       const extraInfo = { oldValue, newValue: value }
       if (!hadKey) {
@@ -60,6 +64,7 @@ function deleteProperty(target: any, key: string | symbol): boolean {
   const oldValue = target[key]
   const result = Reflect.deleteProperty(target, key)
   if (hadKey) {
+    /* istanbul ignore else */
     if (__DEV__) {
       trigger(target, OperationTypes.DELETE, key, { oldValue })
     } else {
@@ -81,8 +86,7 @@ function ownKeys(target: any): (string | number | symbol)[] {
 }
 
 export const mutableHandlers: ProxyHandler<any> = {
-  get: (target: any, key: string | symbol, receiver: any) =>
-    get(target, key, receiver, observable),
+  get: makeGetter(false),
   set,
   deleteProperty,
   has,
@@ -90,8 +94,7 @@ export const mutableHandlers: ProxyHandler<any> = {
 }
 
 export const immutableHandlers: ProxyHandler<any> = {
-  get: (target: any, key: string | symbol, receiver: any) =>
-    get(target, key, receiver, LOCKED ? immutable : observable),
+  get: makeGetter(true),
 
   set(target: any, key: string | symbol, value: any, receiver: any): boolean {
     if (LOCKED) {

packages/observer/src/collectionHandlers.ts

@@ -34,6 +34,7 @@ function add(value: any) {
   const hadKey = proto.has.call(target, value)
   const result = proto.add.call(target, value)
   if (!hadKey) {
+    /* istanbul ignore else */
     if (__DEV__) {
       trigger(target, OperationTypes.ADD, value, { value })
     } else {
@@ -51,6 +52,7 @@ function set(key: any, value: any) {
   const oldValue = proto.get.call(target, key)
   const result = proto.set.call(target, key, value)
   if (value !== oldValue) {
+    /* istanbul ignore else */
     if (__DEV__) {
       const extraInfo = { oldValue, newValue: value }
       if (!hadKey) {
@@ -77,6 +79,7 @@ function deleteEntry(key: any) {
   // forward the operation before queueing reactions
   const result = proto.delete.call(target, key)
   if (hadKey) {
+    /* istanbul ignore else */
     if (__DEV__) {
       trigger(target, OperationTypes.DELETE, key, { oldValue })
     } else {
@@ -94,6 +97,7 @@ function clear() {
   // forward the operation before queueing reactions
   const result = proto.clear.call(target)
   if (hadItems) {
+    /* istanbul ignore else */
     if (__DEV__) {
       trigger(target, OperationTypes.CLEAR, void 0, { oldTarget })
     } else {
@@ -158,22 +162,21 @@ const immutableInstrumentations: any = {
   }
 })
 
-function getInstrumented(
-  target: any,
-  key: string | symbol,
-  receiver: any,
-  instrumentations: any
-) {
-  target = instrumentations.hasOwnProperty(key) ? instrumentations : target
-  return Reflect.get(target, key, receiver)
+function makeInstrumentationGetter(instrumentations: any) {
+  return function getInstrumented(
+    target: any,
+    key: string | symbol,
+    receiver: any
+  ) {
+    target = instrumentations.hasOwnProperty(key) ? instrumentations : target
+    return Reflect.get(target, key, receiver)
+  }
 }
 
 export const mutableCollectionHandlers: ProxyHandler<any> = {
-  get: (target: any, key: string | symbol, receiver: any) =>
-    getInstrumented(target, key, receiver, mutableInstrumentations)
+  get: makeInstrumentationGetter(mutableInstrumentations)
 }
 
 export const immutableCollectionHandlers: ProxyHandler<any> = {
-  get: (target: any, key: string | symbol, receiver: any) =>
-    getInstrumented(target, key, receiver, immutableInstrumentations)
+  get: makeInstrumentationGetter(immutableInstrumentations)
 }