fix(overmind): fix order of entry exists in statecharts

Author: christianalfoni

packages/node_modules/overmind/src/config/statecharts.test.ts

@@ -115,7 +115,7 @@ describe('Statecharts', () => {
     }))
 
     expect(instance.state.states).toEqual([['id1', 'foo']])
-    expect(instance.state.actions).toEqual({ increaseCount: true })
+    expect(instance.state.actions).toEqual({ increaseCount: false })
     expect(instance.state.matches({
       id1: {
         foo: true
@@ -165,7 +165,7 @@ describe('Statecharts', () => {
 
     expect(instance.state.states).toEqual([['id1', 'foo']])
     expect(instance.state.actions).toEqual({
-      increaseCount: true,
+      increaseCount: false,
       changeToBar: true,
     })
     expect(instance.state.matches({
@@ -345,10 +345,10 @@ describe('Statecharts', () => {
 
     expect(instance.state.states).toEqual([['test', 'foo', 'nested', 'a']])
     expect(instance.state.actions).toEqual({
-      fooEntry: true,
-      fooExit: true,
-      aEntry: true,
-      aExit: true,
+      fooEntry: false,
+      fooExit: false,
+      aEntry: false,
+      aExit: false,
       bExit: false,
       changeToBar: true,
       changeToB: true,
@@ -358,11 +358,11 @@ describe('Statecharts', () => {
 
     expect(instance.state.states).toEqual([['test', 'foo', 'nested', 'b']])
     expect(instance.state.actions).toEqual({
-      fooEntry: true,
-      fooExit: true,
+      fooEntry: false,
+      fooExit: false,
       aEntry: false,
       aExit: false,
-      bExit: true,
+      bExit: false,
       changeToBar: true,
       changeToB: false,
     })
@@ -655,7 +655,7 @@ describe('Statecharts', () => {
     const instance = createOvermind(statecharts(config, {mainChart}))
 
     expect(instance.state.states).toEqual([['mainChart', 'foo', 'chart', 'baz'], ['mainChart', 'foo', 'chartB', 'baz']])
-    expect(instance.state.actions).toEqual({ increaseCount: true, changeToBar: true })
+    expect(instance.state.actions).toEqual({ increaseCount: false, changeToBar: true })
     expect(instance.state.matches({
       mainChart: {
         foo: {

packages/node_modules/overmind/src/config/statecharts.ts

@@ -37,60 +37,48 @@ interface Statecharts {
 }
 
 function getActionTransitions(
-  key: string,
+  actionName: string,
   charts: Statecharts,
   state: { states: Array<(string | number)[]> }
 ) {
-  const transitions: (string | boolean)[] = []
-  state.states.forEach((statePath) => {
+  const transitions: Array<{index: number, target?: string}> = []
+
+  state.states.forEach((statePath, index) => {
     const path = statePath.slice()
 
     while (path.length) {
       const target = getStateTarget(charts, path)
 
-      if (target.entry === key) {
-        transitions.push('entry')
-        return
-      }
-      if (target.exit === key) {
-        transitions.push('exit')
+      if (target.on && 
+        target.on[actionName] === null
+      ) {
+        transitions.push({index})
         return
       }
-      if (target.on && target.on[key] === null) {
-        transitions.push(true)
-        return
-      }
-      if (target.on && typeof target.on[key] === 'string') {
-        // If we have already transition to this path it means
-        // we are moving away from multiple paths (parallel)
-        if (transitions.includes(target.on[key])) {
-          return
-        }
-        transitions.push(target.on[key])
+
+      if (target.on && typeof target.on[actionName] === 'string' && !transitions.find((transition) => transition.target === target.on[actionName])) {
+        transitions.push({index, target: target.on[actionName]})
         return
       }
-      if (target.on && target.on[key] && target.on[key].condition(state)) {
-        transitions.push(target.on[key].target)
+
+      if (target.on && target.on[actionName] && target.on[actionName].target && !transitions.find((transition) => transition.target === target.on[actionName].target) && target.on[actionName].condition(state)) {
+        transitions.push({
+          index,
+          target: target.on[actionName].target
+        })
         return
       }
 
       path.pop()
     }
-
-    transitions.push(false)
   })
 
   return transitions
 }
 
-// We check if any transitions returned are not "false",
-// which means any entry, exit or state transition should occur on
-// any of existing state paths active
 function getCanTransitionActions(actions, charts, state) {
   return Object.keys(actions || {}).reduce((aggr, key) => {
-    aggr[key] = getActionTransitions(key, charts, state).some(
-      (transition) => transition !== false
-    )
+    aggr[key] = Boolean(getActionTransitions(key, charts, state).length)
 
     return aggr
   }, {})
@@ -242,20 +230,17 @@ export function statecharts<C extends IConfiguration, Charts extends Statecharts
         context.state,
         context.execution.namespacePath
       )
-      const actions = getTarget(
-        context.actions,
-        context.execution.namespacePath
-      )
 
       const statePaths = stateTarget.states.slice()
 
+      // Run entry actions of initial state
       statePaths.forEach((statePath) => {
         const state = statePath.slice()
         while (state.length) {
           const target = getStateTarget(charts, state)
 
-          if (target.entry) {
-            actions[target.entry]()
+          if (config.actions && config.actions[target.entry]) {
+            config.actions[target.entry](context)
           }
 
           state.pop()
@@ -276,8 +261,7 @@ export function statecharts<C extends IConfiguration, Charts extends Statecharts
     }) as any,
     state: Object.assign(state, {
       states: getInitialStates(charts),
-      actions: ((state) =>
-        getCanTransitionActions(actions, charts, state)) as any,
+      actions: ((state) => getCanTransitionActions(actions, charts, state)) as any,
       matches: (state) => (match) => {
         const matchPaths = getMatchPaths(match)
 
@@ -312,42 +296,39 @@ export function statecharts<C extends IConfiguration, Charts extends Statecharts
           const stateTarget = getTarget(state, execution.namespacePath)
 
           return {
-            transitions: getActionTransitions(key, charts, stateTarget),
+            canTransition: stateTarget.actions[key],
             payload,
           }
         }),
-        filter(function canTransition(_, { transitions }) {
-          return transitions.some((transition) => transition !== false)
+        filter(function canTransition(_, { canTransition }) {
+          return canTransition
         }),
-        mutate(function runAction(context: any, { transitions, payload }) {
+        mutate(function runAction(context: any, { payload }) {
           const stateTarget = getTarget(
             context.state,
             context.execution.namespacePath
           )
-          const actionsTarget = getTarget(
-            context.actions,
-            context.execution.namespacePath
-          )
+          const transitionActions = getActionTransitions(key, charts, stateTarget)
 
-          const resolvedActions = getTarget(
-            context.actions,
-            context.execution.namespacePath
-          )
-          const newStates: Array<(string | number)[]> = []
-
-          if (transitions.includes('entry') || transitions.includes('exit')) {
-            actionsTarget[ACTIONS][key](payload)
+          // If there are no new transition target, just drop moving on, just run the action
+          if (!transitionActions.some((transitionAction) => transitionAction.target)) {
+            if (config.actions) {
+              config.actions[key](context, payload)
+            }
+            return
           }
 
-          const actionsToRun = transitions.reduce((aggr, transition, index) => {
-            const oldStatePath = stateTarget.states[index].slice()
+          const exitActions: string[] = []
+          const entryActions: string[] = []
+          const newStates: Array<string[]> = []
 
-            if (transition === 'entry' || transition === 'exit') {
-              newStates.push(oldStatePath)
-              return aggr
+          transitionActions.forEach((transitionAction) => {
+            // It is an action that does not cause a transition
+            if (!transitionAction.target) {
+              return
             }
 
-            const currentStatePath = stateTarget.states[index].slice()
+            const currentStatePath = stateTarget.states[transitionAction.index].slice()
             const stateTransitions = currentStatePath.map(() => null)
 
             // Build new transition path
@@ -367,20 +348,22 @@ export function statecharts<C extends IConfiguration, Charts extends Statecharts
               stateTarget.states,
               stateTransitions,
               charts,
-              index
+              transitionAction.index
             )
 
             // Go down old path and trigger exits where the state has changed
-            const traverseOldPath = oldStatePath.slice()
+            const traverseOldPath = stateTarget.states[transitionAction.index].slice()
+
             while (traverseOldPath.length) {
               const target = getStateTarget(charts, traverseOldPath)
 
               if (
                 target.exit &&
                 newStatePath[traverseOldPath.length - 1] !==
-                  oldStatePath[traverseOldPath.length - 1]
+                traverseOldPath[traverseOldPath.length - 1]
               ) {
-                resolvedActions[target.exit]()
+                
+                exitActions.push(target.exit)
               }
 
               traverseOldPath.pop()
@@ -396,26 +379,37 @@ export function statecharts<C extends IConfiguration, Charts extends Statecharts
               if (
                 target.entry &&
                 newStatePath[traverseNewPath.length - 1] !==
-                  oldStatePath[traverseNewPath.length - 1]
+                traverseNewPath[traverseNewPath.length - 1]
               ) {
-                resolvedActions[target.entry]()
+                entryActions.push(target.entry)
               }
 
               traverseNewPath.pop()
             }
 
-  
-            if (transition) {
-              return aggr.concat(() => actionsTarget[ACTIONS][key](payload))
-            }
-
-            return aggr
-          }, [] as Function[])
-
-          // Actually change what has changed
+          })
+          
+          // Run exits
+          exitActions.forEach((exitAction) => {
+            if (config.actions) {
+              config.actions[exitAction](context, payload)
+            }  
+          })
+          
+          // Transition to new state
           stateTarget.states = newStates
 
-          actionsToRun.forEach((action)=> action())
+          if (config.actions) {
+            config.actions[key](context, payload)
+          }
+          
+          // Run enters          
+
+          entryActions.forEach((entryAction) => {
+            if (config.actions) {
+              config.actions[entryAction](context, payload)
+            }  
+          })
 
           if (process.env.NODE_ENV === 'development' && currentInstance.devtools) {
             currentInstance.devtools.send({