Merge pull request #67 from mobxjs/type-system-improvements

Type system improvements, fixes #66

src/core/mst-node-administration.ts

@@ -214,10 +214,8 @@ export class MSTAdminisration {
             else
                 current = current!.getChildMST(pathParts[i])
             if (current === null) {
-                if (failIfResolveFails) {
-                    debugger
+                if (failIfResolveFails)
                     return fail(`Could not resolve '${pathParts[i]}' in '${joinJsonPath(pathParts.slice(0, i - 1))}', path of the patch does not resolve`)
-                }
                 else
                     return undefined
             }

src/types/core-types.ts

@@ -1,5 +1,5 @@
 import {IType, Type} from "../core"
-import {invariant, isPrimitive} from "../utils"
+import {invariant, isPrimitive, fail} from "../utils"
 
 export class CoreType<T> extends Type<T, T> {
     readonly checker: (value: any) => boolean
@@ -31,4 +31,19 @@ export const number: IType<number, number> = new CoreType<number>("number", (v:
 // tslint:disable-next-line:variable-name
 export const boolean: IType<boolean, boolean> = new CoreType<boolean>("boolean", (v: any) => typeof v === "boolean")
 // tslint:disable-next-line:variable-name
-export const DatePrimitive: IType<Date, Date> = new CoreType<Date>("Date", (v: any) => v instanceof Date)
+export const DatePrimitive: IType<Date, Date> = new CoreType<Date>("Date", (v: any) => v instanceof Date)
+
+export function getPrimitiveFactoryFromValue(value: any): IType<any, any> {
+    switch (typeof value) {
+        case "string":
+            return string
+        case "number":
+            return number
+        case "boolean":
+            return boolean
+        case "object":
+            if (value instanceof Date)
+                return DatePrimitive
+    }
+    return fail("Cannot determine primtive type from value " + value)
+}

src/types/object.ts

@@ -14,6 +14,7 @@ import { ValueProperty } from "./property-types/value-property"
 import { ActionProperty } from "./property-types/action-property"
 import { getSnapshot } from "../top-level-api"
 import { IJsonPatch } from "../index";
+import { getPrimitiveFactoryFromValue } from "./core-types";
 
 // TODO: make generic with snapshot type
 export interface IObjectInstance {
@@ -92,13 +93,15 @@ export class ObjectType extends ComplexType<any, any> {
             }
 
             const { value } = descriptor
-            if (isIdentifierFactory(value)) {
+            if (value === null || undefined) {
+                fail("The default value of an attribute cannot be null or undefined as the type cannot be inferred. Did you mean `types.maybe(someType)`?")
+            } else if (isIdentifierFactory(value)) {
                 invariant(!this.identifierAttribute, `Cannot define property '${key}' as object identifier, property '${this.identifierAttribute}' is already defined as identifier property`)
                 this.identifierAttribute = key
                 this.props[key] = new IdentifierProperty(key)
             } else if (isPrimitive(value)) {
-                // TODO: detect exact primitiveFactory!
-                this.props[key] = new ValueProperty(key, createDefaultValueFactory(primitiveFactory as any, value))
+                const baseType = getPrimitiveFactoryFromValue(value)
+                this.props[key] = new ValueProperty(key, createDefaultValueFactory(baseType, value))
             } else if (isType(value)) {
                 this.props[key] = new ValueProperty(key, value)
             } else if (isReferenceFactory(value)) {
@@ -157,9 +160,6 @@ export class ObjectType extends ComplexType<any, any> {
     isValidSnapshot(snapshot: any) {
         if (!isPlainObject(snapshot))
             return false
-        for (let key in snapshot)
-            if (!(key in this.props))
-                return false
         for (let key in this.props)
             if (!this.props[key].isValidSnapshot(snapshot))
                 return false

test/array.ts

@@ -163,7 +163,8 @@ test("it should check the type correctly", (t) => {
     t.deepEqual(Factory.is([]), true)
     t.deepEqual(Factory.is({}), false)
     t.deepEqual(Factory.is([{to: 'mars'}]), true)
-    t.deepEqual(Factory.is([{wrongKey: true}]), false)
+    t.deepEqual(Factory.is([{wrongKey: true}]), true)
+    t.deepEqual(Factory.is([{to: true}]), false)
 })
 
 test("it should reconciliate instances correctly", (t) => {

test/map.ts

@@ -164,7 +164,8 @@ test("it should check the type correctly", (t) => {
     t.deepEqual(Factory.is([]), false)
     t.deepEqual(Factory.is({}), true)
     t.deepEqual(Factory.is({hello: {to: 'mars'}}), true)
-    t.deepEqual(Factory.is({hello: {wrongKey: true}}), false)
+    t.deepEqual(Factory.is({hello: {wrongKey: true}}), true)
+    t.deepEqual(Factory.is({hello: {to: true}}), false)
 })
 
 test("it should support identifiers", (t) => {

test/object.ts

@@ -174,7 +174,7 @@ test("it should throw if snapshot has computed properties", (t) => {
         const doc = ComputedFactory.create({area: 3})
     })
 
-    t.is(error.message, "[mobx-state-tree] Snapshot {\"area\":3} is not assignable to type AnonymousModel. Expected { width: primitive; height: primitive } instead.")
+    t.is(error.message, "[mobx-state-tree] Snapshot {\"area\":3} is not assignable to type AnonymousModel. Expected { width: number; height: number } instead.")
 })
 
 // === COMPOSE FACTORY ===
@@ -197,5 +197,6 @@ test("it should check the type correctly", (t) => {
     t.deepEqual(Factory.is([]), false)
     t.deepEqual(Factory.is({}), true)
     t.deepEqual(Factory.is({to: 'mars'}), true)
-    t.deepEqual(Factory.is({wrongKey: true}), false)
+    t.deepEqual(Factory.is({wrongKey: true}), true)
+    t.deepEqual(Factory.is({to: 3 }), false)
 })

test/type-system.ts

@@ -25,12 +25,13 @@ test("it should recognize a valid snapshot", (t) => {
     const { Box } = createTestFactories()
 
     t.deepEqual(Box.is({ width: 1, height: 2 }), true)
+    t.deepEqual(Box.is({ width: 1, height: 2, depth: 3 }), true)
 })
 
 test("it should recognize an invalid snapshot", (t) => {
     const { Box } = createTestFactories()
 
-    t.deepEqual(Box.is({ width: 1, height: 2, depth: 3 }), false)
+    t.deepEqual(Box.is({ width: "1", height: "2" }), false)
 })
 
 test("it should check valid nodes as well", (t) => {
@@ -42,11 +43,11 @@ test("it should check valid nodes as well", (t) => {
 })
 
 test("it should check invalid nodes as well", (t) => {
-    const { Box, Cube } = createTestFactories()
+    const { Box } = createTestFactories()
 
-    const doc = Cube.create()
+    const doc = Box.create()
 
-    t.deepEqual(Box.is(doc), false)
+    t.deepEqual(types.model({ anotherAttr: types.number }).is(doc), false)
 })
 
 test("it should cast different compatible factories", (t) => {
@@ -58,17 +59,16 @@ test("it should cast different compatible factories", (t) => {
 })
 
 test("it should do typescript type inference correctly", (t) => {
-    debugger
     const A = types.model({
         x: types.number,
         y: types.maybe(types.string),
-        method() {},
+        method() { },
         get z(): string { return "hi" },
         set z(v: string) { }
     })
 
     // factory is invokable
-    const a = A.create({ x: 2, y: "7"})
+    const a = A.create({ x: 2, y: "7" })
 
     // property can be used as proper type
     const z: number = a.x
@@ -103,3 +103,86 @@ test("it should do typescript type inference correctly", (t) => {
 
     b.sub.method()
 })
+
+test("#66 - it should accept superfluous fields", t => {
+    const Item = types.model({
+        id: types.number,
+        name: types.string
+    })
+
+    t.is(Item.is({}), false)
+    t.is(Item.is({ id: 3 }), false)
+    t.is(Item.is({ id: 3, name: "" }), true)
+    t.is(Item.is({ id: 3, name: "", description: "" }), true)
+
+    const a = Item.create({ id: 3, name: "", description: "bla" } as any)
+    t.is((a as any).description, undefined)
+})
+
+test("#66 - it should not require defaulted fields", t => {
+    const Item = types.model({
+        id: types.number,
+        name: types.withDefault(types.string, "boo")
+    })
+
+    t.is(Item.is({}), false)
+    t.is(Item.is({ id: 3 }), true)
+    t.is(Item.is({ id: 3, name: "" }), true)
+    t.is(Item.is({ id: 3, name: "", description: "" }), true)
+
+    const a = Item.create({ id: 3, description: "bla" } as any)
+    t.is((a as any).description, undefined)
+    t.is(a.name, "boo")
+})
+
+test("#66 - it should be possible to omit defaulted fields", t => {
+    const Item = types.model({
+        id: types.number,
+        name: "boo"
+    })
+
+    t.is(Item.is({}), false)
+    t.is(Item.is({ id: 3 }), true)
+    t.is(Item.is({ id: 3, name: "" }), true)
+    t.is(Item.is({ id: 3, name: "", description: "" }), true)
+
+    const a = Item.create({ id: 3, description: "bla" } as any)
+    t.is((a as any).description, undefined)
+    t.is(a.name, "boo")
+})
+
+
+test("#66 - it should pick the correct type of defaulted fields", t => {
+    const Item = types.model({
+        id: types.number,
+        name: "boo"
+    })
+
+    const a = Item.create({ id: 3 })
+    t.is(a.name, "boo")
+    t.throws(() => a.name = 3 as any, /Value is not assignable to 'string'/)
+})
+
+test("cannot create factories with null values", t => {
+    t.throws(
+        () => types.model({ x: null }),
+        /The default value of an attribute cannot be null or undefined as the type cannot be inferred. Did you mean `types.maybe\(someType\)`?/
+    )
+})
+
+test("can create factories with maybe primitives", t => {
+    const F = types.model({
+        x: types.maybe(types.string)
+    })
+
+    t.is(F.is(undefined as any), false)
+    t.is(F.is({}), true)
+    t.is(F.is({ x: null }), true)
+    t.is(F.is({ x: "test" }), true)
+    t.is(F.is({ x: 3 }), false)
+
+    t.is(F.create().x, null)
+    t.is(F.create({ x: undefined}).x, null)
+    t.is(F.create({ x: ""}).x, "")
+    t.is(F.create({ x: "3"}).x, "3")
+})

test/union.ts

@@ -3,18 +3,18 @@ import {test} from "ava"
 
 const createTestFactories = () => {
     const Box = types.model("Box", {
-        width: 0,
-        height: 0
+        width: types.number,
+        height: types.number
     })
 
     const Square = types.model("Square", {
-        width: 0
+        width: types.number
     })
 
     const Cube = types.model("Cube", {
-        width: 0,
-        height: 0,
-        depth: 0
+        width: types.number,
+        height: types.number,
+        depth: types.number
     })
 
     const Plane = types.union(Square, Box)
@@ -28,18 +28,18 @@ test("it should complain about no dispatch method", (t) => {
     const {Box, Plane, Square} = createTestFactories()
 
     const error = t.throws(() => {
-        const doc = Plane.create({width: 2})
+        const doc = Plane.create({width: 2, height: 2})
     })
-    t.is(error.message, '[mobx-state-tree] Ambiguos snapshot {"width":2} for union Box | Square. Please provide a dispatch in the union declaration.')
+    t.is(error.message, '[mobx-state-tree] Ambiguos snapshot {"width":2,"height":2} for union Box | Square. Please provide a dispatch in the union declaration.')
 })
 
 test("it should be smart enough to discriminate by keys", (t) => {
     const {Box, Plane, Square} = createTestFactories()
 
-    const doc = Plane.create({height: 1, width: 2})
+    const doc = types.union(Square, Box).create({width: 2})
 
-    t.deepEqual(Box.is(doc), true)
-    t.deepEqual(Square.is(doc), false)
+    t.deepEqual(Box.is(doc), false)
+    t.deepEqual(Square.is(doc), true)
 })
 
 test("it should discriminate by value type", (t) => {
@@ -68,13 +68,13 @@ test("it should discriminate by value type", (t) => {
 test("it should compute exact union types", (t) => {
     const {Box, Plane, Square} = createTestFactories()
 
-    t.deepEqual(Plane.is(Box.create()), true)
-    t.deepEqual(Plane.is(Square.create()), true)
+    t.deepEqual(Plane.is(Box.create({ width: 3, height: 2})), true)
+    t.deepEqual(Plane.is(Square.create({ width: 3})), true)
 })
 
-test("it should compute exact union types", (t) => {
+test("it should compute exact union types - 2", (t) => {
     const {Box, DispatchPlane, Square} = createTestFactories()
 
-    t.deepEqual(DispatchPlane.is(Box.create()), true)
-    t.deepEqual(DispatchPlane.is(Square.create()), true)
+    t.deepEqual(DispatchPlane.is(Box.create({ width: 3, height: 2})), true)
+    t.deepEqual(DispatchPlane.is(Square.create({ width: 3, height: 2} as any)), true)
 })

test/with-default.ts

@@ -33,16 +33,15 @@ test("it should use the snapshot if provided", t => {
 
 test("it should throw if default value is invalid snapshot", t => {
     const Row = types.model({
-        name: '',
-        quantity: 0
+        name: types.string,
+        quantity: types.number
     })
 
     const error = t.throws(() => {
         const Factory = types.model({
-            // TODO: as any due to #19
-            rows: types.withDefault(types.array(Row) as any, [{wrongProp: true}])
+            rows: types.withDefault(types.array(Row), [{}])
         })
     })
 
-    t.is(error.message, '[mobx-state-tree] Default value [{"wrongProp":true}] is not assignable to type AnonymousModel[]. Expected "{ name: primitive; quantity: primitive }[]"')
+    t.is(error.message, "[mobx-state-tree] Default value [{}] is not assignable to type AnonymousModel[]. Expected \"{ name: string; quantity: number }[]\"")
 })