toggle hooks in middleware

API.md

@@ -33,6 +33,7 @@ _This reference guide lists all methods exposed by MST. Contributions like lingu
 -   [getSnapshot](#getsnapshot)
 -   [getType](#gettype)
 -   [hasParent](#hasparent)
+-   [hooks](#hooks)
 -   [Identifier](#identifier)
 -   [IdentifierCache](#identifiercache)
 -   [isAlive](#isalive)
@@ -124,7 +125,11 @@ For more details, see the [middleware docs](docs/middleware.md)
 **Parameters**
 
 -   `target` **IStateTreeNode** 
--   `middleware`  
+-   `middleware`  **IMiddleware**
+-   `includeHooks` **([boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean) | any)** indicates whether the [hooks](#hooks) should be piped to the middleware.
+
+See [hooks](#hooks) for more information.
+
 
 Returns **IDisposer** 
 
@@ -207,7 +212,7 @@ Binds middleware to a specific action
 
 **Parameters**
 
--   `middleware` **IMiddlewareHandler** 
+-   `handler` **IMiddlewareHandler** 
 -   `fn`  
 -   `Function`  } fn
 
@@ -385,6 +390,27 @@ Given a model instance, returns `true` if the object has a parent, that is, is p
 
 Returns **[boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)** 
 
+## hooks
+
+The current action based LifeCycle hooks for [types.model](#types.model).
+
+```typescript
+enum HookNames {
+    afterCreate = "afterCreate",
+    afterAttach = "afterAttach",
+    postProcessSnapshot = "postProcessSnapshot",
+    beforeDetach = "beforeDetach",
+    beforeDestroy = "beforeDestroy"
+}
+```
+
+Note: 
+Unlike the other hooks, `preProcessSnapshot` is not created as part of the actions initializer, but directly on the type.
+`preProcessSnapshot` is currently not piped to middlewares. 
+
+For more details on middlewares, see the [middleware docs](docs/middleware.md) 
+
+
 ## Identifier
 
 ## IdentifierCache

README.md

@@ -906,13 +906,13 @@ See the [full API docs](API.md) for more details.
 | signature | |
 | ---- | --- |
 | [`addDisposer(node, () => void)`](API.md#adddisposer) | Function to be invoked whenever the target node is to be destroyed |
-| [`addMiddleware(node, middleware: (actionDescription, next) => any)`](API.md#addmiddleware) | Attaches middleware to a node. See [middleware](docs/middleware.md). Returns disposer. |
+| [`addMiddleware(node, middleware: (actionDescription, next) => any, includeHooks)`](API.md#addmiddleware) | Attaches middleware to a node. See [middleware](docs/middleware.md). Returns disposer. |
 | [`applyAction(node, actionDescription)`](API.md#applyaction) | Replays an action on the targeted node |
 | [`applyPatch(node, jsonPatch)`](API.md#applypatch) | Applies a JSON patch, or array of patches, to a node in the tree |
 | [`applySnapshot(node, snapshot)`](API.md#applysnapshot) | Updates a node with the given snapshot |
 | [`createActionTrackingMiddleware`](API.md#createactiontrackingmiddleware) | Utility to make writing middleware that tracks async actions less cumbersome |
 | [`clone(node, keepEnvironment?: true \| false \| newEnvironment)`](API.md#clone) | Creates a full clone of the given node. By default preserves the same environment |
-| [`decorate(middleware, function)`](API.md#decorate) | Attaches middleware to a specific action (or flow) |
+| [`decorate(handler, function)`](API.md#decorate) | Attaches middleware to a specific action (or flow) |
 | [`destroy(node)`](API.md#destroy) | Kills `node`, making it unusable. Removes it from any parent in the process |
 | [`detach(node)`](API.md#detach) | Removes `node` from its current parent, and lets it live on as standalone tree |
 | [`flow(generator)`](API.md#flow) | creates an asynchronous flow based on a generator function |

packages/mobx-state-tree/src/core/action.ts

@@ -7,7 +7,8 @@ import {
     IDisposer,
     getRoot,
     EMPTY_ARRAY,
-    ObjectNode
+    ObjectNode,
+    HookNames
 } from "../internal"
 
 export type IMiddlewareEventType =
@@ -30,6 +31,10 @@ export type IMiddlewareEvent = {
     args: any[]
 }
 
+export type IMiddleware = {
+    handler: IMiddlewareHandler
+    includeHooks: boolean
+}
 export type IMiddlewareHandler = (
     actionCall: IMiddlewareEvent,
     next: (actionCall: IMiddlewareEvent) => any
@@ -96,15 +101,19 @@ export function createActionInvoker<T extends Function>(
  * @param {(action: IRawActionCall, next: (call: IRawActionCall) => any) => any} middleware
  * @returns {IDisposer}
  */
-export function addMiddleware(target: IStateTreeNode, middleware: IMiddlewareHandler): IDisposer {
+export function addMiddleware(
+    target: IStateTreeNode,
+    middleware: IMiddlewareHandler,
+    includeHooks: boolean = true
+): IDisposer {
     const node = getStateTreeNode(target)
     if (process.env.NODE_ENV !== "production") {
         if (!node.isProtectionEnabled)
             console.warn(
                 "It is recommended to protect the state tree before attaching action middleware, as otherwise it cannot be guaranteed that all changes are passed through middleware. See `protect`"
             )
     }
-    return node.addMiddleWare(middleware)
+    return node.addMiddleWare(middleware, includeHooks)
 }
 
 export function decorate<T extends Function>(middleware: IMiddlewareHandler, fn: T): T
@@ -126,41 +135,54 @@ export function decorate<T extends Function>(middleware: IMiddlewareHandler, fn:
  *
  * @export
  * @template T
- * @param {IMiddlewareHandler} middleware
+ * @param {IMiddlewareHandler} handler
  * @param Function} fn
  * @returns the original function
  */
-export function decorate<T extends Function>(middleware: IMiddlewareHandler, fn: any) {
+export function decorate<T extends Function>(handler: IMiddlewareHandler, fn: any) {
+    const middleware: IMiddleware = { handler, includeHooks: true }
     if (fn.$mst_middleware) fn.$mst_middleware.push(middleware)
     else fn.$mst_middleware = [middleware]
     return fn
 }
 
-function collectMiddlewareHandlers(
+function collectMiddlewares(
     node: ObjectNode,
     baseCall: IMiddlewareEvent,
     fn: Function
-): IMiddlewareHandler[] {
-    let handlers: IMiddlewareHandler[] = (fn as any).$mst_middleware || EMPTY_ARRAY
+): IMiddleware[] {
+    let middlewares: IMiddleware[] = (fn as any).$mst_middleware || EMPTY_ARRAY
     let n: ObjectNode | null = node
     // Find all middlewares. Optimization: cache this?
     while (n) {
-        if (n.middlewares) handlers = handlers.concat(n.middlewares)
+        if (n.middlewares) middlewares = middlewares.concat(n.middlewares)
         n = n.parent
     }
-    return handlers
+    return middlewares
 }
 
 function runMiddleWares(node: ObjectNode, baseCall: IMiddlewareEvent, originalFn: Function): any {
-    const handlers = collectMiddlewareHandlers(node, baseCall, originalFn)
+    const middlewares = collectMiddlewares(node, baseCall, originalFn)
     // Short circuit
-    if (!handlers.length) return mobxAction(originalFn).apply(null, baseCall.args)
+    if (!middlewares.length) return mobxAction(originalFn).apply(null, baseCall.args)
     let index = 0
 
     function runNextMiddleware(call: IMiddlewareEvent): any {
-        const handler = handlers[index++]
-        if (handler) return handler(call, runNextMiddleware)
-        else return mobxAction(originalFn).apply(null, baseCall.args)
+        const middleware = middlewares[index++]
+        const handler = middleware && middleware.handler
+        const invokeHandler = () => {
+            const next = handler(call, runNextMiddleware)
+            return next
+        }
+
+        if (handler && middleware.includeHooks) {
+            return invokeHandler()
+        } else if (handler && !middleware.includeHooks) {
+            if ((HookNames as any)[call.name]) return runNextMiddleware(call)
+            return invokeHandler()
+        } else {
+            return mobxAction(originalFn).apply(null, baseCall.args)
+        }
     }
     return runNextMiddleware(baseCall)
 }

packages/mobx-state-tree/src/core/node/object-node.ts

@@ -15,6 +15,7 @@ import {
     registerEventHandler,
     addReadOnlyProp,
     walk,
+    IMiddleware,
     IMiddlewareHandler,
     createActionInvoker,
     NodeLifeCycle,
@@ -47,7 +48,7 @@ export class ObjectNode implements INode {
     protected _autoUnbox = true // unboxing is disabled when reading child nodes
     state = NodeLifeCycle.INITIALIZING
 
-    middlewares = EMPTY_ARRAY as IMiddlewareHandler[]
+    middlewares = EMPTY_ARRAY as IMiddleware[]
     private snapshotSubscribers: ((snapshot: any) => void)[]
     private patchSubscribers: ((patch: IJsonPatch, reversePatch: IJsonPatch) => void)[]
     private disposers: (() => void)[]
@@ -404,8 +405,15 @@ export class ObjectNode implements INode {
         this.disposers.unshift(disposer)
     }
 
-    addMiddleWare(handler: IMiddlewareHandler) {
-        return registerEventHandler(this.middlewares, handler)
+    removeMiddleware(handler: IMiddlewareHandler) {
+        this.middlewares = this.middlewares.filter(middleware => middleware.handler !== handler)
+    }
+
+    addMiddleWare(handler: IMiddlewareHandler, includeHooks: boolean = true) {
+        this.middlewares.push({ handler, includeHooks })
+        return () => {
+            this.removeMiddleware(handler)
+        }
     }
 
     applyPatchLocally(subpath: string, patch: IJsonPatch): void {

packages/mobx-state-tree/src/types/complex-types/model.ts

@@ -42,12 +42,12 @@ import {
 
 const PRE_PROCESS_SNAPSHOT = "preProcessSnapshot"
 
-const HOOK_NAMES = {
-    afterCreate: "afterCreate",
-    afterAttach: "afterAttach",
-    postProcessSnapshot: "postProcessSnapshot",
-    beforeDetach: "beforeDetach",
-    beforeDestroy: "beforeDestroy"
+export enum HookNames {
+    afterCreate = "afterCreate",
+    afterAttach = "afterAttach",
+    postProcessSnapshot = "postProcessSnapshot",
+    beforeDetach = "beforeDetach",
+    beforeDestroy = "beforeDestroy"
 }
 
 function objectTypeToString(this: any) {
@@ -72,7 +72,7 @@ function toPropertiesObject<T>(properties: IModelProperties<T>): { [K in keyof T
     return Object.keys(properties).reduce(
         (properties, key) => {
             // warn if user intended a HOOK
-            if (key in HOOK_NAMES)
+            if (key in HookNames)
                 return fail(
                     `Hook '${key}' was defined as property. Hooks should be defined as part of the actions`
                 )
@@ -169,9 +169,9 @@ export class ModelType<S, T> extends ComplexType<S, T> implements IModelType<S,
             // apply hook composition
             let action = actions[name]
             let baseAction = (self as any)[name]
-            if (name in HOOK_NAMES && baseAction) {
+            if (name in HookNames && baseAction) {
                 let specializedAction = action
-                if (name === HOOK_NAMES.postProcessSnapshot)
+                if (name === HookNames.postProcessSnapshot)
                     action = (snapshot: any) => specializedAction(baseAction(snapshot))
                 else
                     action = function() {
@@ -381,8 +381,9 @@ export class ModelType<S, T> extends ComplexType<S, T> implements IModelType<S,
             ;(extras.getAtom(node.storedValue, name) as any).reportObserved()
             res[name] = this.getChildNode(node, name).snapshot
         })
-        if (typeof node.storedValue.postProcessSnapshot === "function")
+        if (typeof node.storedValue.postProcessSnapshot === "function") {
             return node.storedValue.postProcessSnapshot.call(null, res)
+        }
         return res
     }
 

packages/mst-middlewares/src/undo-manager.ts

@@ -134,7 +134,7 @@ const UndoManager = types
                     throw new Error(
                         "UndoManager should be created as part of a tree, or with `targetStore` in it's environment"
                     )
-                middlewareDisposer = addMiddleware(targetStore, undoRedoMiddleware)
+                middlewareDisposer = addMiddleware(targetStore, undoRedoMiddleware, false)
             },
             beforeDestroy() {
                 middlewareDisposer()

packages/mst-middlewares/test/TimeTraveller.ts

@@ -1,6 +1,6 @@
 import { test } from "ava"
 import { TimeTraveller } from "../src"
-import { types, addMiddleware, flow, clone } from "mobx-state-tree"
+import { types, flow, clone } from "mobx-state-tree"
 
 const TestModel = types
     .model({

packages/mst-middlewares/test/custom.ts

@@ -0,0 +1,65 @@
+import { test } from "ava"
+import atomic from "../src/atomic"
+import { types, addMiddleware, getSnapshot } from "mobx-state-tree"
+
+let error: any = null
+
+function customMiddleware1(call, next) {
+    // omit next() when there are more middlewares in the queue
+    //return next(call)
+}
+function customMiddleware2(call, next) {
+    return next(call)
+}
+
+function noHooksMiddleware(call, next) {
+    // thowing errors will lead to the aborting of further middlewares
+    // => don't throw here but set a global var instead
+    if (call.name === "postProcessSnapshot") error = Error("hook in middleware")
+    return next(call)
+}
+
+const TestModel = types
+    .model("Test1", {
+        z: 1
+    })
+    .actions(self => {
+        return {
+            inc(x) {
+                self.z += x
+                return self.z
+            },
+            postProcessSnapshot(snapshot) {
+                return snapshot
+            }
+        }
+    })
+
+test("next() omitted within middleware", t => {
+    const m = TestModel.create()
+    addMiddleware(m, customMiddleware1)
+    addMiddleware(m, customMiddleware2)
+    let thrownError: any = null
+    try {
+        m.inc(1)
+    } catch (e) {
+        thrownError = e
+    }
+    t.is(!!thrownError, true)
+})
+
+test("middleware should be invoked on hooks", t => {
+    const m = TestModel.create()
+    error = null
+    addMiddleware(m, noHooksMiddleware)
+    m.inc(1)
+    t.is(!!error, true)
+})
+
+test("middleware should not be invoked on hooks", t => {
+    const m = TestModel.create()
+    error = null
+    addMiddleware(m, noHooksMiddleware, false)
+    m.inc(1)
+    t.is(!error, true)
+})