Implemented view functions, improved typings

README.md

@@ -164,6 +164,8 @@ TODO
 
 Views versus actions
 
+Exception: `"Invariant failed: Side effects like changing state are not allowed at this point."` indicates that a view function tries to modifies a model. This is only allowed in actions.
+
 ## Protecting the state tree
 
 By default it is allowed to both directly modify a model or through an action.

src/types/complex-types/object.ts

@@ -216,8 +216,8 @@ export type Snapshot<T> = {
 
 export interface IModelType<T, A> extends IComplexType<Snapshot<T>, T & A> { }
 
-export function createModelFactory<T>(baseModel: IBaseModelDefinition<T>): IModelType<T, {}>
-export function createModelFactory<T>(name: string, baseModel: IBaseModelDefinition<T>): IModelType<T, {}>
+export function createModelFactory<T>(baseModel: IBaseModelDefinition<T> & ThisType<T>): IModelType<T, {}>
+export function createModelFactory<T>(name: string, baseModel: IBaseModelDefinition<T> & ThisType<T>): IModelType<T, {}>
 export function createModelFactory<T, A>(baseModel: IBaseModelDefinition<T> & ThisType<T>, actions: A & ThisType<T & A>): IModelType<T, A>
 export function createModelFactory<T, A>(name: string, baseModel: IBaseModelDefinition<T> & ThisType<T>, actions: A & ThisType<T & A>): IModelType<T, A>
 export function createModelFactory(arg1: any, arg2?: any, arg3?: any) {

src/types/property-types/view-property.ts

@@ -1,21 +1,34 @@
-import { addHiddenFinalProp } from "../../utils"
-import { createActionInvoker } from "../../core"
+import { extras } from "mobx"
+import { addHiddenFinalProp, createNamedFunction } from "../../utils"
+import { IMSTNode, getMSTAdministration } from "../../core"
 import { Property } from "./property"
 
 export class ViewProperty extends Property {
-    invokeAction: Function
+    invokeView: Function
 
     constructor(name: string, fn: Function) {
         super(name)
-        this.invokeAction = createActionInvoker(name, fn)
-        throw new Error("oops")
+        this.invokeView = createViewInvoker(name, fn)
     }
 
     initialize(target: any) {
-        addHiddenFinalProp(target, this.name, this.invokeAction.bind(target))
+        addHiddenFinalProp(target, this.name, this.invokeView.bind(target))
     }
 
     isValidSnapshot(snapshot: any) {
         return !(this.name in snapshot)
     }
 }
+
+export function createViewInvoker(name: string, fn: Function) {
+    const viewInvoker = function (this: IMSTNode) {
+        const args = arguments
+        const adm = getMSTAdministration(this)
+        adm.assertAlive()
+        return extras.allowStateChanges(false, () => fn.apply(this, args))
+    }
+
+    // This construction helps producing a better function name in the stack trace, but could be optimized
+    // away in prod builds, and `actionInvoker` be returned directly
+    return createNamedFunction(name, viewInvoker)
+}

test/object.ts

@@ -1,5 +1,6 @@
 import {onSnapshot, onPatch, onAction, applyPatch, applyPatches, applyAction, applyActions, getPath, IJsonPatch, applySnapshot, getSnapshot, types} from "../"
 import {test} from "ava"
+import {autorun} from "mobx"
 
 interface ITestSnapshot{
     to: string
@@ -237,3 +238,43 @@ test("it should check the type correctly", (t) => {
     t.deepEqual(Factory.is({wrongKey: true}), true)
     t.deepEqual(Factory.is({to: 3 }), false)
 })
+
+// === VIEW FUNCTIONS ===
+
+test("view functions should be tracked", (t) => {
+    const model = types.model({
+        x: 3,
+        doubler() {
+            return this.x * 2
+        }
+    }).create()
+
+    const values: number[] = []
+    const d = autorun(() => {
+        values.push(model.doubler())
+    })
+
+    model.x = 7
+    t.deepEqual(values, [6, 14])
+})
+
+test("view functions should not be allowed to change state", (t) => {
+    const model = types.model({
+        x: 3,
+        doubler() {
+            this.x *= 2
+        }
+    }, {
+        anotherDoubler() {
+            this.x *= 2
+        }
+    }).create()
+
+    t.throws(
+        () => model.doubler(),
+        /Invariant failed: Side effects like changing state are not allowed at this point. Are you trying to modify state from, for example, the render function of a React component?/
+    )
+
+    model.anotherDoubler()
+    t.is(model.x, 6)
+})

test/type-system.ts

@@ -191,7 +191,8 @@ test("can create factories with maybe primitives", t => {
 
 test("it is possible to refer to a type", t => {
     const Todo = types.model({
-        title: types.string,
+        title: types.string
+    }, {
         setTitle(v: string) {
 
         }
@@ -211,7 +212,8 @@ test("it is possible to refer to a type", t => {
 
 test(".Type should not be callable", t => {
     const Todo = types.model({
-        title: types.string,
+        title: types.string
+    }, {
         setTitle(v: string) {
 
         }
@@ -223,7 +225,8 @@ test(".Type should not be callable", t => {
 
 test(".SnapshotType should not be callable", t => {
     const Todo = types.model({
-        title: types.string,
+        title: types.string
+    }, {
         setTitle(v: string) {
 
         }
@@ -233,10 +236,10 @@ test(".SnapshotType should not be callable", t => {
 })
 
 test("types instances with compatible snapshots should not be interchangeable", t => {
-    const A = types.model("A", {
+    const A = types.model("A", {}, {
         doA() {}
     })
-    const B = types.model("B", {
+    const B = types.model("B", {}, {
         doB() {}
     })
     const C = types.model("C", {
@@ -257,3 +260,31 @@ test("types instances with compatible snapshots should not be interchangeable",
     //     "[mobx-state-tree] Value of type B: '{}' is not assignable to type: A | null, expected an instance of A | null or a snapshot like '({  } | null)' instead. (Note that a snapshot of the provided value is compatible with the targeted type)"
     // )
 })
+
+test("it handles complex types correctly", t => {
+    const Todo = types.model({
+        title: types.string
+    }, {
+        setTitle(v: string) {
+
+        }
+    })
+
+    const Store = types.model({
+        todos: types.map(Todo),
+        get amount() {
+            // double check, not available design time:
+            /// this.setAmount()
+            return this.todos.size
+        },
+        getAmount(): number {
+            return this.todos.size + this.amount
+        }
+    }, {
+        setAmount() {
+            const x: number = this.todos.size + this.amount + this.getAmount
+        }
+    })
+
+    t.is(true, true) // supress no asserts warning
+})