Made map / array values optional for model properties, implements #906

mobxjs · Jul 4, 2018 · a8c7c60 · a8c7c60
1 parent 85efcd2
commit a8c7c60
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 32 deletions.
diff --git a/changelog.md b/changelog.md
@@ -2,6 +2,7 @@
 
 ## Improvements
 
+* It is no longer necessary to wrap `types.map` or `types.array` in `types.optional` when used in a model type, they are now optional by default when used as property type.
 * Introduced `setLivelynessChecking("warn" | "ignore" | "error")`
 // TODO: add actual numbers
 * Significantly improved the performance of constructing MST trees. Significantly reduced the memory footprint of MST. Big shoutout to the relentless effort by [k-g-a](https://github.com/k-g-a) to optimize all the things! See [#845](https://github.com/mobxjs/mobx-state-tree/issues/845) for details.

diff --git a/packages/mobx-state-tree/src/types/complex-types/model.ts b/packages/mobx-state-tree/src/types/complex-types/model.ts
@@ -10,7 +10,8 @@ import {
     _getAdministration,
     isComputedProp,
     computed,
-    set
+    set,
+    IObservableArray
 } from "mobx"
 import {
     fail,
@@ -49,7 +50,12 @@ import {
     IAnyType,
     Union,
     Late,
-    OptionalValue
+    OptionalValue,
+    isMapType,
+    isArrayType,
+    IMSTMap,
+    MapType,
+    ArrayType
 } from "../../internal"
 
 const PRE_PROCESS_SNAPSHOT = "preProcessSnapshot"
@@ -84,9 +90,17 @@ export type ModelPropertiesDeclarationToProperties<T extends ModelPropertiesDecl
                 ? IType<boolean | undefined, boolean, boolean> & { flags: TypeFlags.Optional }
                 : T[K] extends Date
                     ? IType<number | undefined, number, Date> & { flags: TypeFlags.Optional }
-                    : T[K] extends IType<infer C, infer S, infer T> & { flags: TypeFlags.Optional }
-                        ? IType<C, S, T> & { flags: TypeFlags.Optional }
-                        : T[K] extends IType<infer C, infer S, infer T> ? IType<C, S, T> : never
+                    : T[K] extends (IComplexType<
+                          any,
+                          any,
+                          IMSTMap<any, any, any> | IObservableArray<any>
+                      >)
+                        ? T[K] & { flags: TypeFlags.Optional }
+                        : T[K] extends IType<any, any, any> & {
+                              flags: TypeFlags.Optional
+                          }
+                            ? T[K] & { flags: TypeFlags.Optional }
+                            : T[K] extends IType<any, any, any> ? T[K] : never
 }
 
 export type OptionalPropertyTypes = ModelPrimitive | { flags: TypeFlags.Optional }
@@ -205,6 +219,13 @@ function toPropertiesObject<T>(declaredProps: ModelPropertiesDeclaration): Model
                 return Object.assign({}, props, {
                     [key]: optional(getPrimitiveFactoryFromValue(value), value)
                 })
+                // map defaults to empty object automatically for models
+            } else if (value instanceof MapType) {
+                return Object.assign({}, props, {
+                    [key]: optional(value, {})
+                })
+            } else if (value instanceof ArrayType) {
+                return Object.assign({}, props, { [key]: optional(value, []) })
                 // its already a type
             } else if (isType(value)) {
                 return props
@@ -695,7 +716,7 @@ export function compose(...args: any[]): any {
         .named(typeName)
 }
 
-export function isObjectType(type: any): type is ModelType<any, any> {
+export function isModelType(type: any): type is ModelType<any, any> {
     return isType(type) && (type.flags & TypeFlags.Object) > 0
 }
 

diff --git a/packages/mobx-state-tree/test/array.ts b/packages/mobx-state-tree/test/array.ts
@@ -167,7 +167,7 @@ test("items should be reconciled correctly when splicing - 1", () => {
         d = Task.create({ x: "d" })
     const store = types
         .model({
-            todos: types.optional(types.array(Task), [])
+            todos: types.array(Task)
         })
         .create({
             todos: [a]
@@ -295,7 +295,7 @@ test("it correctly reconciliate when swapping", () => {
 test("it correctly reconciliate when swapping using snapshots", () => {
     const Task = types.model("Task", {})
     const Store = types.model({
-        todos: types.optional(types.array(Task), [])
+        todos: types.array(Task)
     })
     const s = Store.create()
     unprotect(s)

diff --git a/packages/mobx-state-tree/test/late.ts b/packages/mobx-state-tree/test/late.ts
@@ -30,7 +30,7 @@ test("late should describe correctly circular references", () => {
     const Node = types.model("Node", {
         childs: types.array(types.late(() => Node))
     })
-    expect(Node.describe()).toEqual("{ childs: Node[] }")
+    expect(Node.describe()).toEqual("{ childs: Node[]? }")
 })
 test("should typecheck", () => {
     const NodeObject = types.model("NodeObject", {

diff --git a/packages/mobx-state-tree/test/map.ts b/packages/mobx-state-tree/test/map.ts
@@ -287,7 +287,7 @@ test("it should not throw when removing a non existing item from a map", () => {
     expect(() => {
         const AppModel = types
             .model({
-                myMap: types.optional(types.map(types.number), {})
+                myMap: types.map(types.number)
             })
             .actions(self => {
                 function something() {

diff --git a/packages/mobx-state-tree/test/object.ts b/packages/mobx-state-tree/test/object.ts
@@ -159,10 +159,10 @@ test("it should apply snapshots", () => {
     expect(getSnapshot(doc)).toEqual({ to: "universe" })
 })
 test("it should apply and accept null value for types.maybe(complexType)", () => {
-    const Item = types.model({
+    const Item = types.model("Item", {
         value: types.string
     })
-    const Model = types.model({
+    const Model = types.model("Model", {
         item: types.maybe(Item)
     })
     const myModel = Model.create()
@@ -462,7 +462,7 @@ test("it should check the type correctly", () => {
 })
 
 if (process.env.NODE_ENV !== "production") {
-    test("it should require complex fields to be present", () => {
+    test("complex map / array values are optional by default", () => {
         expect(
             types
                 .model({
@@ -475,36 +475,40 @@ if (process.env.NODE_ENV !== "production") {
                 .model({
                     todo: types.model({})
                 })
-                .create()
+                .create({} as any)
         ).toThrow()
         expect(
             types
                 .model({
                     todo: types.array(types.string)
                 })
                 .is({})
-        ).toBe(false) // TBD: or true?
-        expect(() =>
-            types
-                .model({
-                    todo: types.array(types.string)
-                })
-                .create()
-        ).toThrow()
+        ).toBe(true) // TBD: or true?
+        expect(
+            getSnapshot(
+                types
+                    .model({
+                        todo: types.array(types.string)
+                    })
+                    .create({})
+            )
+        ).toEqual({ todo: [] })
         expect(
             types
                 .model({
                     todo: types.map(types.string)
                 })
                 .is({})
-        ).toBe(false)
-        expect(() =>
-            types
-                .model({
-                    todo: types.map(types.string)
-                })
-                .create()
-        ).toThrow()
+        ).toBe(true)
+        expect(
+            getSnapshot(
+                types
+                    .model({
+                        todo: types.map(types.string)
+                    })
+                    .create({})
+            )
+        ).toEqual({ todo: {} })
     })
 }
 // === VIEW FUNCTIONS ===

diff --git a/packages/mobx-state-tree/test/reference.ts b/packages/mobx-state-tree/test/reference.ts
@@ -141,7 +141,7 @@ test("it should resolve refs over late types", () => {
         }))
     const Store = types.model({
         books: types.array(Book),
-        entries: types.optional(types.array(BookEntry), [])
+        entries: types.array(BookEntry)
     })
     const s = Store.create({
         books: [{ id: "3", price: 2 }]

diff --git a/packages/mobx-state-tree/test/reflection.ts b/packages/mobx-state-tree/test/reflection.ts
@@ -7,7 +7,7 @@ const Model = types
     .model({
         isPerson: false,
         users: types.optional(types.map(User), {}),
-        dogs: types.optional(types.array(User), []),
+        dogs: types.array(User),
         user: types.maybe(types.late(() => User))
     })
     .volatile(self => ({