Implemented protection-by-default, fixes mobxjs#101

README.md

@@ -161,6 +161,11 @@ Useful methods:
 
 It is not necessary to express all logic around models as actions. For example it is not possible to define constructors on models. Rather, it is recommended to create stateless utility methods that operate on your models. It is recommended to keep models self-contained and to do orchestration around models in utilities around it.
 
+
+## (Un) protecting state tree
+
+`afterCreate() { unprotect(this) }`
+
 ## Views
 
 TODO

changelog.md

@@ -1,3 +1,7 @@
+# 0.5.0
+
+** BREAKING ** protection enabled by default
+
 # 0.4.0
 
 **BREAKING** `types.model` no requires 2 parameters to define a model. The first parameter defines the properties, derived values and view functions. The second argment is used to define the actions. For example:

src/core/mst-node-administration.ts

@@ -1,7 +1,6 @@
 import {
     action, observable,
-    computed, reaction,
-    IReactionDisposer
+    computed, reaction
 } from "mobx"
 import { typecheck, IType } from "../types/type"
 import { isMST, getMSTAdministration } from "./mst-node"
@@ -22,10 +21,10 @@ export class MSTAdministration {
     @observable _parent: MSTAdministration | null = null
     @observable subpath: string = ""
     readonly type: ComplexType<any, any>
+    isProtectionEnabled = true
     _environment: any = undefined
     _isRunningAction = false // only relevant for root
     private _isAlive = true // optimization: use binary flags for all these switches
-    private _isProtected = false
     private _isDetaching = false
 
     readonly middlewares: IMiddleWareHandler[] = []
@@ -84,6 +83,7 @@ export class MSTAdministration {
         if (this._isDetaching)
             return
 
+        // tslint:disable-next-line:no_unused-variable
         this.type.getChildMSTs(this).forEach(([_, node]) => {
             node.die()
         })
@@ -116,15 +116,16 @@ export class MSTAdministration {
     }
 
     public applySnapshot(snapshot: any) {
-        this.assertWritable()
         typecheck(this.type, snapshot)
         return this.type.applySnapshot(this, snapshot)
     }
 
     @action public applyPatch(patch: IJsonPatch) {
         const parts = splitJsonPath(patch.path)
         const node = this.resolvePath(parts.slice(0, -1))
-        node.applyPatchLocally(parts[parts.length - 1], patch)
+        node.pseudoAction(() => {
+            node.applyPatchLocally(parts[parts.length - 1], patch)
+        })
     }
 
     applyPatchLocally(subpath: string, patch: IJsonPatch): void {
@@ -297,15 +298,22 @@ export class MSTAdministration {
     get isProtected(): boolean {
         let cur: MSTAdministration | null = this
         while (cur) {
-            if (cur._isProtected === true)
-                return true
+            if (cur.isProtectionEnabled === false)
+                return false
             cur = cur.parent
         }
-        return false
+        return true
     }
 
-    protect() {
-        this._isProtected = true
+    /**
+     * Pseudo action is an action that is not named, does not trigger middleware but does unlock the tree.
+     * Used for applying (initial) snapshots and patches
+     */
+    pseudoAction(fn: () => void) {
+        const inAction = this._isRunningAction
+        this._isRunningAction = true
+        fn()
+        this._isRunningAction = inAction
     }
 
     assertWritable() {

src/core/mst-operations.ts

@@ -50,7 +50,7 @@ import { ISnapshottable, IType } from "../types/type"
  */
 export function addMiddleware(target: IMSTNode, middleware: (action: IRawActionCall, next: (call: IRawActionCall) => any) => any): IDisposer {
     const node = getMSTAdministration(target)
-    if (!node.isProtected)
+    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)
 }
@@ -186,14 +186,18 @@ export function recordActions(subject: IMSTNode): IActionRecorder {
  * todo.toggle() // OK
  */
 export function protect(target: IMSTNode) {
-    getMSTAdministration(target).protect()
+    getMSTAdministration(target).isProtectionEnabled = true
+}
+
+export function unprotect(target: IMSTNode) {
+    getMSTAdministration(target).isProtectionEnabled = false
 }
 
 /**
  * Returns true if the object is in protected mode, @see protect
  */
 export function isProtected(target: IMSTNode): boolean {
-    return getMSTAdministration(target).isProtected
+    return getMSTAdministration(target).isProtectionEnabled
 }
 
 /**

src/types/complex-types/array.ts

@@ -128,8 +128,10 @@ export class ArrayType<T> extends ComplexType<T[], IObservableArray<T>> {
     }
 
     @action applySnapshot(node: MSTAdministration, snapshot: any[]): void {
-        const target = node.target as IObservableArray<any>
-        target.replace(snapshot)
+        node.pseudoAction(() => {
+            const target = node.target as IObservableArray<any>
+            target.replace(snapshot)
+        })
     }
 
     getChildType(key: string): IType<any, any> {

src/types/complex-types/complex-type.ts

@@ -15,7 +15,9 @@ export abstract class ComplexType<S, T> extends Type<S, T> {
         const instance = this.createNewInstance()
         // tslint:disable-next-line:no_unused-variable
         const node = new MSTAdministration(parent, subpath, instance, this, environment)
-        this.finalizeNewInstance(instance, snapshot)
+        node.pseudoAction(() => {
+            this.finalizeNewInstance(instance, snapshot)
+        })
         node.fireHook("afterCreate")
         if (parent)
             node.fireHook("afterAttach")

src/types/complex-types/map.ts

@@ -138,35 +138,37 @@ export class MapType<S, T> extends ComplexType<{[key: string]: S}, IExtendedObse
     }
 
     @action applySnapshot(node: MSTAdministration, snapshot: any): void {
-        const target = node.target as ObservableMap<any>
-        const identifierAttr = getIdentifierAttribute(this.subType)
-        // Try to update snapshot smartly, by reusing instances under the same key as much as possible
-        const currentKeys: { [key: string]: boolean } = {}
-        target.keys().forEach(key => { currentKeys[key] = false })
-        Object.keys(snapshot).forEach(key => {
-            const item = snapshot[key]
-            if (identifierAttr && item && typeof item === "object" && key !== item[identifierAttr])
-                fail(`A map of objects containing an identifier should always store the object under their own identifier. Trying to store key '${key}', but expected: '${item[identifierAttr]}'`)
-            // if snapshot[key] is non-primitive, and this.get(key) has a Node, update it, instead of replace
-            if (key in currentKeys && !isPrimitive(item)) {
-                currentKeys[key] = true
-                maybeMST(
-                    target.get(key),
-                    propertyNode => {
-                        // update existing instance
-                        propertyNode.applySnapshot(item)
-                    },
-                    () => {
-                        target.set(key, item)
-                    }
-                )
-            } else {
-                target.set(key, item)
-            }
-        })
-        Object.keys(currentKeys).forEach(key => {
-            if (currentKeys[key] === false)
-                target.delete(key)
+        node.pseudoAction(() => {
+            const target = node.target as ObservableMap<any>
+            const identifierAttr = getIdentifierAttribute(this.subType)
+            // Try to update snapshot smartly, by reusing instances under the same key as much as possible
+            const currentKeys: { [key: string]: boolean } = {}
+            target.keys().forEach(key => { currentKeys[key] = false })
+            Object.keys(snapshot).forEach(key => {
+                const item = snapshot[key]
+                if (identifierAttr && item && typeof item === "object" && key !== item[identifierAttr])
+                    fail(`A map of objects containing an identifier should always store the object under their own identifier. Trying to store key '${key}', but expected: '${item[identifierAttr]}'`)
+                // if snapshot[key] is non-primitive, and this.get(key) has a Node, update it, instead of replace
+                if (key in currentKeys && !isPrimitive(item)) {
+                    currentKeys[key] = true
+                    maybeMST(
+                        target.get(key),
+                        propertyNode => {
+                            // update existing instance
+                            propertyNode.applySnapshot(item)
+                        },
+                        () => {
+                            target.set(key, item)
+                        }
+                    )
+                } else {
+                    target.set(key, item)
+                }
+            })
+            Object.keys(currentKeys).forEach(key => {
+                if (currentKeys[key] === false)
+                    target.delete(key)
+            })
         })
     }
 

src/types/complex-types/object.ts

@@ -160,10 +160,10 @@ export class ObjectType extends ComplexType<any, any> {
     }
 
     @action applySnapshot(node: MSTAdministration, snapshot: any): void {
-        // TODO:fix: all props should be processed when applying snapshot, and reset to default if needed
-        for (let key in snapshot) if (key in this.props) {
+        // TODO:fix: all props should be processed when applying snapshot, and reset to default if needed?
+        node.pseudoAction(() => { for (let key in snapshot) if (key in this.props) {
             this.props[key].deserialize(node.target, snapshot)
-        }
+        }})
     }
 
     getChildType(key: string): IType<any, any> {