[Internal] Cleanup 'createNode' and related codepaths

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

@@ -1,56 +1,28 @@
-import {
-    isStateTreeNode,
-    INode,
-    identity,
-    noop,
-    fail,
-    ObjectNode,
-    ScalarNode,
-    IType
-} from "../../internal"
+import { INode, fail, ObjectNode, ScalarNode, IType, getStateTreeNodeSafe } from "../../internal"
 
-// TODO: split into object and scalar node?
 export function createNode<C, S, T>(
     type: IType<C, S, T>,
     parent: ObjectNode | null,
     subpath: string,
     environment: any,
-    initialValue: any,
-    createNewInstance: (initialValue: any) => T = identity,
-    finalizeNewInstance: (node: INode, childNodes?: any) => void = noop
+    initialValue: any
 ) {
-    if (isStateTreeNode(initialValue)) {
-        const targetNode = initialValue.$treenode as ObjectNode
-        if (!targetNode.isRoot)
-            fail(
-                `Cannot add an object to a state tree if it is already part of the same or another state tree. Tried to assign an object to '${
-                    parent ? parent.path : ""
-                }/${subpath}', but it lives already at '${targetNode.path}'`
-            )
-        targetNode.setParent(parent, subpath)
-        return targetNode
-    }
+    const existingNode = getStateTreeNodeSafe(initialValue)
+    if (existingNode) {
+        if (existingNode.isRoot) {
+            existingNode.setParent(parent, subpath)
+            return existingNode
+        }
 
-    if (type.shouldAttachNode) {
-        return new ObjectNode(
-            type,
-            parent,
-            subpath,
-            environment,
-            initialValue,
-            createNewInstance,
-            finalizeNewInstance
+        fail(
+            `Cannot add an object to a state tree if it is already part of the same or another state tree. Tried to assign an object to '${
+                parent ? parent.path : ""
+            }/${subpath}', but it lives already at '${existingNode.path}'`
         )
     }
-    return new ScalarNode(
-        type,
-        parent,
-        subpath,
-        environment,
-        initialValue,
-        createNewInstance,
-        finalizeNewInstance
-    )
+
+    const Node = type.shouldAttachNode ? ObjectNode : ScalarNode
+    return new Node(type, parent, subpath, environment, initialValue)
 }
 
 export function isNode(value: any): value is INode {

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

@@ -70,6 +70,10 @@ export function getStateTreeNode(value: IAnyStateTreeNode): ObjectNode {
     else return fail(`Value ${value} is no MST Node`)
 }
 
+export function getStateTreeNodeSafe(value: IAnyStateTreeNode): ObjectNode {
+    return (value && value.$treenode) || null
+}
+
 export function canAttachNode(value: any) {
     return (
         value &&

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

@@ -5,6 +5,7 @@ import {
     ComplexType,
     convertChildNodesToArray,
     createActionInvoker,
+    EMPTY_OBJECT,
     escapeJsonPath,
     extend,
     fail,
@@ -58,8 +59,6 @@ const snapshotReactionOptions = {
     }
 }
 
-const NOT_FILLED_REF = {}
-
 export class ObjectNode implements INode {
     nodeId = ++nextNodeId
     readonly type: IAnyType
@@ -97,25 +96,19 @@ export class ObjectNode implements INode {
     private _snapshotSubscribers: ((snapshot: any) => void)[] | null = null
 
     private _observableInstanceCreated: boolean = false
-    private _childNodes: IChildNodesMap | null
+    private _childNodes: IChildNodesMap
     private _initialSnapshot: any
     private _cachedInitialSnapshot: any = null
-    private _createNewInstance: ((initialValue: any) => any) | null
-    private _finalizeNewInstance: ((node: INode, initialValue: any) => void) | null
 
     constructor(
         type: IAnyType,
         parent: ObjectNode | null,
         subpath: string,
         environment: any,
-        initialSnapshot: any,
-        createNewInstance: (initialValue: any) => any,
-        finalizeNewInstance: (node: INode, initialValue: any) => void
+        initialSnapshot: any
     ) {
         this._environment = environment
         this._initialSnapshot = freeze(initialSnapshot)
-        this._createNewInstance = createNewInstance
-        this._finalizeNewInstance = finalizeNewInstance
 
         this.type = type
         this.parent = parent
@@ -144,17 +137,15 @@ export class ObjectNode implements INode {
 
     @action
     private _createObservableInstance() {
-        this.storedValue = this._createNewInstance!(this._childNodes)
+        const type = this.type
+        this.storedValue = type.createNewInstance(this, this._childNodes, this._initialSnapshot)
         this.preboot()
 
-        addHiddenFinalProp(this.storedValue, "$treenode", this)
-        addHiddenFinalProp(this.storedValue, "toJSON", toJSON)
-
-        this._observableInstanceCreated = true
         let sawException = true
+        this._observableInstanceCreated = true
         try {
             this._isRunningAction = true
-            this._finalizeNewInstance!(this, this._childNodes)
+            type.finalizeNewInstance(this, this.storedValue)
             this._isRunningAction = false
 
             this.fireHook("afterCreate")
@@ -174,9 +165,7 @@ export class ObjectNode implements INode {
 
         this.finalizeCreation()
 
-        this._childNodes = null
-        this._createNewInstance = null
-        this._finalizeNewInstance = null
+        this._childNodes = EMPTY_OBJECT
     }
 
     /*
@@ -248,12 +237,8 @@ export class ObjectNode implements INode {
         if (typeof fn === "function") fn.apply(this.storedValue)
     }
 
-    public get value() {
+    public get value(): any {
         if (!this._observableInstanceCreated) this._createObservableInstance()
-        return this._value
-    }
-
-    private get _value(): any {
         if (!this.isAlive) return undefined
         return this.type.getValue(this)
     }
@@ -432,6 +417,9 @@ export class ObjectNode implements INode {
                 return self.type.applySnapshot(self, snapshot)
             }
         )
+
+        addHiddenFinalProp(this.storedValue, "$treenode", this)
+        addHiddenFinalProp(this.storedValue, "toJSON", toJSON)
     }
 
     @action

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

@@ -4,7 +4,6 @@ import {
     fail,
     freeze,
     NodeLifeCycle,
-    noop,
     ObjectNode,
     IAnyType
 } from "../../internal"
@@ -24,21 +23,17 @@ export class ScalarNode implements INode {
         parent: ObjectNode | null,
         subpath: string,
         environment: any,
-        initialSnapshot: any,
-        createNewInstance: (initialValue: any) => any,
-        finalizeNewInstance: (node: INode, initialValue: any) => void = noop
+        initialSnapshot: any
     ) {
         this._initialSnapshot = initialSnapshot
 
         this.type = type
         this.parent = parent
         this.subpath = subpath
-        this.storedValue = createNewInstance(initialSnapshot)
 
         let sawException = true
         try {
-            finalizeNewInstance(this, initialSnapshot)
-
+            this.storedValue = type.createNewInstance(this, {}, initialSnapshot)
             this.state = NodeLifeCycle.CREATED
             sawException = false
         } finally {

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

@@ -18,8 +18,7 @@ import {
     ObjectNode,
     IChildNodesMap,
     ModelPrimitive,
-    IReferenceType,
-    isArray
+    IReferenceType
 } from "../../internal"
 
 export enum TypeFlags {
@@ -57,7 +56,9 @@ export interface IType<C, S, T> {
 
     // Internal api's
     instantiate(parent: INode | null, subpath: string, environment: any, initialValue?: any): INode
-    initializeChildNodes(node: INode, snapshot: any): IChildNodesMap | null
+    initializeChildNodes(node: INode, snapshot: any): IChildNodesMap
+    createNewInstance(node: INode, childNodes: IChildNodesMap, snapshot: any): any
+    finalizeNewInstance(node: INode, instance: any): void
     reconcile(current: INode, newValue: any): INode
     getValue(node: INode): T
     getSnapshot(node: INode, applyPostProcess?: boolean): S
@@ -150,10 +151,16 @@ export abstract class ComplexType<C, S, T> implements IType<C, S, T> {
         typecheck(this, snapshot)
         return this.instantiate(null, "", environment, snapshot).value
     }
-    initializeChildNodes(node: INode, snapshot: any): IChildNodesMap | null {
-        return null
+    initializeChildNodes(node: INode, snapshot: any): IChildNodesMap {
+        return {}
     }
 
+    createNewInstance(node: INode, childNodes: IChildNodesMap, snapshot: any): any {
+        return snapshot
+    }
+
+    finalizeNewInstance(node: INode, instance: any) {}
+
     abstract instantiate(
         parent: INode | null,
         subpath: string,

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

@@ -1,46 +1,45 @@
 import {
-    observable,
-    IObservableArray,
-    IArrayWillChange,
-    IArrayWillSplice,
+    _getAdministration,
+    action,
     IArrayChange,
     IArraySplice,
-    action,
+    IArrayWillChange,
+    IArrayWillSplice,
     intercept,
-    observe,
-    _getAdministration
+    IObservableArray,
+    observable,
+    observe
 } from "mobx"
 import {
+    ComplexType,
+    convertChildNodesToArray,
     createNode,
+    EMPTY_ARRAY,
+    fail,
+    flattenTypeErrors,
+    getContextForPath,
     getStateTreeNode,
+    IAnyType,
+    IChildNodesMap,
+    IComplexType,
+    IContext,
     IJsonPatch,
     INode,
+    isArray,
+    isMutable,
+    isNode,
+    isPlainObject,
     isStateTreeNode,
     IStateTreeNode,
-    isNode,
-    typecheck,
-    flattenTypeErrors,
-    getContextForPath,
-    IContext,
-    IValidationResult,
-    typeCheckFailure,
-    ComplexType,
-    IComplexType,
-    IType,
     isType,
-    fail,
-    isMutable,
-    isArray,
-    isPlainObject,
-    TypeFlags,
-    ObjectNode,
+    IType,
+    IValidationResult,
     mobxShallow,
-    IChildNodesMap,
-    convertChildNodesToArray,
-    IAnyType,
-    EMPTY_ARRAY
+    ObjectNode,
+    typecheck,
+    typeCheckFailure,
+    TypeFlags
 } from "../../internal"
-import { ModelType } from "./model"
 
 export interface IMSTArray<C, S, T> extends IObservableArray<T> {}
 export interface IArrayType<C, S, T>
@@ -58,33 +57,8 @@ export class ArrayType<C, S, T> extends ComplexType<C[] | undefined, S[], IMSTAr
         this.subType = subType
     }
 
-    describe() {
-        return this.subType.describe() + "[]"
-    }
-
-    createNewInstance(childNodes: IChildNodesMap) {
-        return observable.array(convertChildNodesToArray(childNodes), mobxShallow)
-    }
-
-    finalizeNewInstance(node: INode) {
-        const objectNode = node as ObjectNode
-        const type = objectNode.type as ArrayType<any, any, any>
-        const instance = objectNode.storedValue as IObservableArray<any>
-        _getAdministration(instance).dehancer = objectNode.unbox
-        intercept(instance, type.willChange as any)
-        observe(instance, type.didChange)
-    }
-
     instantiate(parent: ObjectNode | null, subpath: string, environment: any, snapshot: S): INode {
-        return createNode(
-            this,
-            parent,
-            subpath,
-            environment,
-            snapshot,
-            this.createNewInstance,
-            this.finalizeNewInstance
-        )
+        return createNode(this, parent, subpath, environment, snapshot)
     }
 
     initializeChildNodes(objNode: ObjectNode, snapshot: S[] = []): IChildNodesMap {
@@ -98,6 +72,23 @@ export class ArrayType<C, S, T> extends ComplexType<C[] | undefined, S[], IMSTAr
         return result
     }
 
+    createNewInstance(
+        node: ObjectNode,
+        childNodes: IChildNodesMap,
+        snapshot: any
+    ): IObservableArray<any> {
+        return observable.array(convertChildNodesToArray(childNodes), mobxShallow)
+    }
+    finalizeNewInstance(node: ObjectNode, instance: IObservableArray<any>): void {
+        _getAdministration(instance).dehancer = node.unbox
+        intercept(instance, this.willChange as any)
+        observe(instance, this.didChange)
+    }
+
+    describe() {
+        return this.subType.describe() + "[]"
+    }
+
     getChildren(node: ObjectNode): INode[] {
         return node.storedValue.slice()
     }