feat(overmind): send pending and error state of effects, also send flushes correctly

christianalfoni · christianalfoni · commit f174d1acec3d · 2019-01-06T20:07:43.000+01:00
diff --git a/packages/node_modules/overmind/src/index.ts b/packages/node_modules/overmind/src/index.ts
@@ -16,6 +16,7 @@ import {
   Options,
   ResolveActions,
   ResolveState,
+  Execution,
 } from './internalTypes'
 import { proxifyEffects } from './proxyfyEffects'
 import {
@@ -108,10 +109,6 @@ export class Overmind<Config extends Configuration> implements Configuration {
     this.proxyStateTree = proxyStateTree
     this.eventHub = eventHub
 
-    /*
-      The action factory function
-    */
-
     if (!IS_PRODUCTION && typeof window !== 'undefined') {
       let warning = 'OVERMIND: You are running in DEVELOPMENT mode.'
       if (options.logProxies !== true) {
@@ -139,32 +136,50 @@ export class Overmind<Config extends Configuration> implements Configuration {
       console.warn(warning)
     }
 
-    eventHub.on(EventType.OPERATOR_ASYNC, (data) => {
-      const flushData = proxyStateTree.flush(true)
-      if (this.devtools && flushData.mutations.length) {
-        this.devtools.send({
-          type: 'flush',
-          data: {
-            ...data,
-            ...flushData,
-            mutations: makeStringifySafeMutations(flushData.mutations),
-          },
-        })
-      }
-    })
-    eventHub.on(EventType.ACTION_END, (data) => {
-      const flushData = proxyStateTree.flush()
-      if (this.devtools && flushData.mutations.length) {
-        this.devtools.send({
-          type: 'flush',
-          data: {
-            ...data,
-            ...flushData,
-            mutations: makeStringifySafeMutations(flushData.mutations),
-          },
-        })
+    if (IS_PRODUCTION) {
+      eventHub.on(EventType.OPERATOR_ASYNC, () => {
+        proxyStateTree.mutationTree.flush()
+      })
+      eventHub.on(EventType.ACTION_END, () => {
+        proxyStateTree.mutationTree.flush()
+      })
+
+      let nextTick
+      const flushTree = () => {
+        proxyStateTree.mutationTree.flush(true)
       }
-    })
+
+      this.proxyStateTree.onMutation(() => {
+        nextTick && clearTimeout(nextTick)
+        nextTick = setTimeout(flushTree, 0)
+      })
+    } else {
+      eventHub.on(EventType.OPERATOR_ASYNC, (execution) => {
+        const flushData = execution.flush()
+        if (this.devtools && flushData) {
+          this.devtools.send({
+            type: 'flush',
+            data: {
+              ...execution,
+              ...flushData,
+            },
+          })
+        }
+      })
+      eventHub.on(EventType.ACTION_END, (execution) => {
+        const flushData = execution.flush()
+
+        if (this.devtools && flushData) {
+          this.devtools.send({
+            type: 'flush',
+            data: {
+              ...execution,
+              ...flushData,
+            },
+          })
+        }
+      })
+    }
 
     /*
       Expose the created actions
@@ -181,21 +196,18 @@ export class Overmind<Config extends Configuration> implements Configuration {
     } else {
       this.initialized = Promise.resolve(null)
     }
-
-    if (IS_PRODUCTION) {
-      let nextTick
-      const flushTree = () => {
-        proxyStateTree.flush(true)
-      }
-
-      this.proxyStateTree.onMutation(() => {
-        nextTick && clearTimeout(nextTick)
-        nextTick = setTimeout(flushTree, 0)
-      })
-    }
   }
   private createExecution(name, action) {
-    return {
+    if (IS_PRODUCTION) {
+      return {
+        getMutationTree: () => {
+          return this.proxyStateTree.mutationTree
+        },
+      } as Execution
+    }
+
+    const mutationTrees: any[] = []
+    const execution = {
       actionId: this.actionReferences.indexOf(action),
       executionId: this.nextExecutionId++,
       actionName: name,
@@ -204,7 +216,19 @@ export class Overmind<Config extends Configuration> implements Configuration {
       emit: this.eventHub.emit.bind(this.eventHub),
       send: this.devtools ? this.devtools.send.bind(this.devtools) : () => {},
       trackEffects: this.trackEffects.bind(this, this.effects),
+      flush: () => {
+        return this.proxyStateTree.flush(mutationTrees)
+      },
+      getMutationTree: () => {
+        const mutationTree = this.proxyStateTree.getMutationTree()
+
+        mutationTrees.push(mutationTree)
+
+        return mutationTree
+      },
     }
+
+    return execution
   }
   private createContext(value, execution, tree) {
     return Object.assign(
@@ -250,15 +274,15 @@ export class Overmind<Config extends Configuration> implements Configuration {
                     value,
                     state: this.proxyStateTree.state,
                     execution,
-                    proxyStateTree: this.proxyStateTree,
                   },
                   this.trackEffects(this.effects, execution)
                 ),
                 (err, finalContext) => {
-                  this.eventHub.emit(EventType.ACTION_END, {
-                    ...finalContext.execution,
-                    operatorId: finalContext.execution.operatorId - 1,
-                  })
+                  finalContext &&
+                    this.eventHub.emit(EventType.ACTION_END, {
+                      ...finalContext.execution,
+                      operatorId: finalContext.execution.operatorId - 1,
+                    })
                   if (err) reject(err)
                   else resolve(finalContext.value)
                 }
@@ -267,7 +291,7 @@ export class Overmind<Config extends Configuration> implements Configuration {
                 this.createContext(
                   value,
                   execution,
-                  this.proxyStateTree.getMutationTree()
+                  execution.getMutationTree()
                 )
               )
         })
@@ -280,16 +304,18 @@ export class Overmind<Config extends Configuration> implements Configuration {
         this.eventHub.emit(EventType.ACTION_START, execution)
         this.eventHub.emit(EventType.OPERATOR_START, execution)
 
-        const mutationTree = this.proxyStateTree.getMutationTree()
+        const mutationTree = execution.getMutationTree()
 
         mutationTree.onMutation((mutation) => {
           this.eventHub.emit(EventType.MUTATIONS, {
             ...execution,
             mutations: makeStringifySafeMutations([mutation]),
           })
+
           setTimeout(() => {
-            const flushData = this.proxyStateTree.flush(true)
-            if (this.devtools && flushData.mutations.length) {
+            const flushData = mutationTree.flush(true)
+
+            if (this.devtools && flushData) {
               this.devtools.send({
                 type: 'flush',
                 data: {
@@ -316,16 +342,6 @@ export class Overmind<Config extends Configuration> implements Configuration {
         })
         this.eventHub.emit(EventType.ACTION_END, execution)
 
-        const flushData = this.proxyStateTree.flush()
-        if (this.devtools && flushData.mutations.length) {
-          this.devtools.send({
-            type: 'flush',
-            data: {
-              ...execution,
-              ...flushData,
-            },
-          })
-        }
         return Promise.resolve(result)
       }
     }
@@ -336,7 +352,7 @@ export class Overmind<Config extends Configuration> implements Configuration {
     }
 
     return proxifyEffects(this.effects, (effect) =>
-      this.eventHub.emitAsync(EventType.EFFECT, { ...execution, ...effect })
+      this.eventHub.emit(EventType.EFFECT, { ...execution, ...effect })
     )
   }
   private initializeDevtools(host, eventHub, proxyStateTree) {
@@ -775,16 +791,16 @@ export function action<Input, Config extends Configuration = TheConfig>(
     else {
       startDebugOperator('action', operation, context)
 
-      const mutationTree = context.proxyStateTree.getMutationTree()
+      const mutationTree = context.execution.getMutationTree()
       if (!IS_PRODUCTION) {
         mutationTree.onMutation((mutation) => {
           context.execution.emit(EventType.MUTATIONS, {
             ...context.execution,
             mutations: makeStringifySafeMutations([mutation]),
           })
           setTimeout(() => {
-            const flushData = context.proxyStateTree.flush(true)
-            if (flushData.mutations.length) {
+            const flushData = mutationTree.flush(true)
+            if (flushData) {
               context.execution.send({
                 type: 'flush',
                 data: {
diff --git a/packages/node_modules/overmind/src/internalTypes.ts b/packages/node_modules/overmind/src/internalTypes.ts
@@ -1,4 +1,4 @@
-import { IMutation, ProxyStateTree } from 'proxy-state-tree'
+import { IMutation, IMutationTree } from 'proxy-state-tree'
 import { Configuration, TAction, TOperator, TStateObject } from './types'
 
 export type SubType<Base, Condition> = Pick<
@@ -36,7 +36,12 @@ export type Execution = {
   actionName: string
   operatorId: number
   path: string[]
-  emit: (event: EventType, value: any) => void
+  emit(event: EventType, value: any): void
+  flush(): {
+    mutations: IMutation[]
+    flushId: number
+  }
+  getMutationTree(): IMutationTree<any>
 }
 
 export interface Events {
@@ -70,7 +75,15 @@ export interface Events {
     path: string
     flushId: number
   }
-  [EventType.EFFECT]: any
+  [EventType.EFFECT]: Execution & {
+    result: any
+    name: string
+    method: string
+    args: any[]
+    isPending: boolean
+    error: string
+    effectId: number
+  }
   [EventType.REACTION_ADD]: {
     path: string
     statePath: string
diff --git a/packages/node_modules/overmind/src/pipe.test.ts b/packages/node_modules/overmind/src/pipe.test.ts
@@ -26,8 +26,6 @@ function createMockAction(pipe) {
           emit: () => {},
           send: () => {},
           trackEffects: () => {},
-        },
-        proxyStateTree: {
           getMutationTree() {
             return {
               onMutation() {},
diff --git a/packages/node_modules/overmind/src/proxyfyEffects.ts b/packages/node_modules/overmind/src/proxyfyEffects.ts
@@ -5,27 +5,70 @@ function isObject(value) {
 }
 
 let hasWarnedConstructor = false
+let currentEffectId = 0
 
 function wrapEffect(target, prop, path, cb) {
   const func = function(...args) {
-    const result = target[prop].apply(this, args)
+    const effectId = currentEffectId++
+    let result
+    try {
+      result = target[prop].apply(this, args)
+    } catch (error) {
+      // eslint-disable-next-line standard/no-callback-literal
+      cb({
+        effectId,
+        name: path,
+        method: prop,
+        args,
+        isPending: false,
+        error: error.message,
+      })
+      return
+    }
+
     if (result instanceof Promise) {
-      result.then((promisedResult) => {
-        // eslint-disable-next-line standard/no-callback-literal
-        cb({
-          name: path,
-          method: prop,
-          args,
-          result: promisedResult,
-        })
+      // eslint-disable-next-line standard/no-callback-literal
+      cb({
+        effectId,
+        name: path,
+        method: prop,
+        args,
+        isPending: true,
+        error: false,
       })
+      result
+        .then((promisedResult) => {
+          // eslint-disable-next-line standard/no-callback-literal
+          cb({
+            effectId,
+            name: path,
+            method: prop,
+            args,
+            result: promisedResult,
+            isPending: false,
+            error: false,
+          })
+        })
+        .catch((error) => {
+          cb({
+            effectId,
+            name: path,
+            method: prop,
+            args,
+            isPending: false,
+            error: error.message,
+          })
+        })
     } else {
       // eslint-disable-next-line standard/no-callback-literal
       cb({
+        effectId,
         name: path,
         method: prop,
         args,
         result,
+        isPending: false,
+        error: false,
       })
     }
     return result
