feat(overmind): strict mode using state machines

Author: christianalfoni

packages/node_modules/overmind/src/index.ts

@@ -11,6 +11,7 @@ import {
   ProxyStateTree,
   TTree,
   VALUE,
+  PROXY_TREE,
 } from 'proxy-state-tree'
 
 import { Derived, IS_DERIVED, IS_DERIVED_CONSTRUCTOR } from './derived'
@@ -191,6 +192,7 @@ export class Overmind<ThisConfig extends IConfiguration>
   private mode: DefaultMode | TestMode | SSRMode
   private reydrateMutationsForHotReloading: IMutation[] = []
   private originalConfiguration
+  private isStrict = false
   initialized: Promise<any>
   eventHub: EventEmitter<Events>
   devtools: Devtools
@@ -209,6 +211,7 @@ export class Overmind<ThisConfig extends IConfiguration>
     const devEnv = options.devEnv || 'development'
 
     this.delimiter = options.delimiter || '.'
+    this.isStrict = Boolean(options.strict)
 
     if (
       (!process.env.NODE_ENV || process.env.NODE_ENV === devEnv) &&
@@ -238,7 +241,7 @@ export class Overmind<ThisConfig extends IConfiguration>
     const proxyStateTree = this.createProxyStateTree(
       configuration,
       eventHub,
-      mode.mode === MODE_SSR ? false : process.env.NODE_ENV === devEnv
+      mode.mode === MODE_TEST || process.env.NODE_ENV === devEnv
     )
     this.originalConfiguration = configuration
     this.state = proxyStateTree.state
@@ -574,8 +577,12 @@ export class Overmind<ThisConfig extends IConfiguration>
             )
           })
         } else {
+          const mutationTree = execution.getMutationTree()
+          if (this.isStrict) {
+            mutationTree.blockMutations()
+          }
           const returnValue = action(
-            this.createContext(execution, execution.getMutationTree()),
+            this.createContext(execution, mutationTree),
             value
           )
 
@@ -596,7 +603,9 @@ export class Overmind<ThisConfig extends IConfiguration>
         this.eventHub.emit(EventType.OPERATOR_START, execution)
 
         const mutationTree = execution.getMutationTree()
-
+        if (this.isStrict) {
+          mutationTree.blockMutations()
+        }
         mutationTree.onMutation((mutation) => {
           this.eventHub.emit(EventType.MUTATIONS, {
             ...execution,

packages/node_modules/overmind/src/internalTypes.ts

@@ -24,6 +24,7 @@ export type Options = {
   devtools?: string | boolean
   logProxies?: boolean
   hotReloading?: boolean
+  strict?: boolean
 }
 
 export type DefaultMode = {

packages/node_modules/overmind/src/statemachine.test.ts

@@ -205,7 +205,7 @@ describe('Statemachine', () => {
     return overmind.actions.transition()
   })
 
-  test('should enable mutations after new async matching or transition', async () => {
+  test('should only enable mutations with matches', async () => {
     expect.assertions(3)
 
     type States = {
@@ -224,13 +224,13 @@ describe('Statemachine', () => {
       await state.transition('BAR', {}, async () => {
         await Promise.resolve()
         expect(state[PROXY_TREE].master.mutationTree.isBlocking).toBe(true)
-        if (state.matches('BAR')) {
+        state.matches('BAR', async () => {
           expect(state[PROXY_TREE].master.mutationTree.isBlocking).toBe(false)
           await Promise.resolve()
           state.transition('FOO', {}, () => {
-            expect(state[PROXY_TREE].master.mutationTree.isBlocking).toBe(false)
+            expect(state[PROXY_TREE].master.mutationTree.isBlocking).toBe(true)
           })
-        }
+        })
       })
     }
 
@@ -390,4 +390,70 @@ describe('Statemachine', () => {
 
     overmind.actions.transition()
   })
+  test('should not allow mutations in strict mode', () => {
+    type States = {
+      current: 'FOO'
+    } | {
+      current: 'BAR'
+    }
+
+    const state = statemachine<States>({
+      FOO: ['BAR'],
+      BAR: ['FOO']
+    }, {
+      current: 'FOO',
+    })
+    const transition: Action = ({ state }) => {
+      state.current = 'BAR'
+    }
+
+    const config = {
+      state,
+      actions: {
+        transition
+      }
+    } 
+
+    interface Action extends IAction<typeof config, void, void> {}
+
+    const overmind = createOvermindMock(config)
+    // @ts-ignore
+    overmind.isStrict = true
+    
+
+    expect(() => overmind.actions.transition()).toThrow()
+  })
+  test('should allow mutations using transition', () => {
+    type States = {
+      current: 'FOO'
+    } | {
+      current: 'BAR'
+    }
+
+    const state = statemachine<States>({
+      FOO: ['BAR'],
+      BAR: ['FOO']
+    }, {
+      current: 'FOO',
+    })
+    const transition: Action = ({ state }) => {
+      state.transition('BAR', {})
+    }
+
+    const config = {
+      state,
+      actions: {
+        transition
+      }
+    } 
+
+    interface Action extends IAction<typeof config, void, void> {}
+
+    const overmind = createOvermindMock(config)
+    // @ts-ignore
+    overmind.isStrict = true
+    
+
+    expect(() => overmind.actions.transition()).not.toThrow()
+  })
 })

packages/node_modules/overmind/src/statemachine.ts

@@ -31,9 +31,9 @@ export type StatemachineTransitions<States extends TStates> = {
 }
 
 export interface MachineMethods<States extends TStates, Base extends IState = {}> {
-  matches<T extends States["current"]>(...state: T[]): this is Statemachine<Base, States, States extends {
+  matches<T extends States["current"], O = void>(state: T | T[], cb: (current: Statemachine<Base, States, States extends {
     current: T
-  } ? States : never>;
+  } ? States : never>) => O): O;
   transition<T extends States["current"], O = void>(
     state: T,
     newState: States extends {
@@ -70,30 +70,31 @@ export class StateMachine<Base extends IState, States extends TStates, State ext
     this[CURRENT_KEYS] = Object.keys(state)
     Object.assign(this, state, base)
   }
-  matches(...states) {
+  matches(states, cb) {
+    states = Array.isArray(states) ? states : [states]
+
     if (states.includes(this.current)) {
       const tree = (this[PROXY_TREE].master.mutationTree || this[PROXY_TREE])
 
-      if (tree.enableMutations) {
-        tree.enableMutations()
-        Promise.resolve().then(() => {
-          tree.blockMutations()
-        })
-  
+      if (!tree.enableMutations) {   
+        throw new Error('Overmind - The "matches" API is only to be used for state changes in actions, point to "state.current" to check current state of machine')
       }
-      return true
-    }
 
-    return false
+      tree.enableMutations()
+      const result = cb(this)
+      tree.blockMutations()
+      return result
+    }
   }
   transition(state, newState, callback) {
     const transitions = this[VALUE][TRANSITIONS] 
-    
+
     if (transitions[this.current].includes(state)) {
       const tree = (this[PROXY_TREE].master.mutationTree || this[PROXY_TREE])
       const baseKeys = Object.keys(this[VALUE][BASE])
 
       tree.enableMutations()
+
       this[VALUE][CURRENT_KEYS].forEach((key) => {
         if (!baseKeys.includes(key)) {
           delete this[key]
@@ -104,13 +105,13 @@ export class StateMachine<Base extends IState, States extends TStates, State ext
       this.current = state
       this[VALUE][CURRENT_KEYS] = Object.keys(newState)
 
+      tree.blockMutations()
+
       let result
       if (callback) {
         result = callback(this)
       }
 
-      tree.blockMutations()
-
       return result
     } else if (process.env.NODE_ENV === 'development' && state !== this.current) {
       console.warn(`Overmind Statemachine - You tried to transition into "${state}", but it is not a valid transition. The valid transitions are ${JSON.stringify(transitions[this.current])}`)

packages/node_modules/proxy-state-tree/src/Proxyfier.ts

@@ -194,7 +194,6 @@ export class Proxifier {
         )
 
         const mutationTree = proxifier.getMutationTree()
-        const existingValue = target[prop]
         const result = Reflect.set(target, prop, value)
 
         mutationTree.addMutation({
@@ -333,8 +332,6 @@ export class Proxifier {
         }
 
         const mutationTree = proxifier.getMutationTree()
-        const existingValue = target[prop]
-
 
         delete target[prop]