Fix issue fast initializing an array from undefined

gadget-inc · Nov 7, 2023 · e7c370b · e7c370b
1 parent efa91fb
commit e7c370b
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 23 deletions.
diff --git a/spec/class-model.spec.ts b/spec/class-model.spec.ts
@@ -12,14 +12,13 @@ import type {
   ModelPropertiesDeclaration,
   SnapshotIn,
 } from "../src";
-import { isType } from "../src";
-import { flow, getSnapshot, getType, isReadOnlyNode, isStateTreeNode, types } from "../src";
+import { flow, getSnapshot, getType, isReadOnlyNode, isStateTreeNode, isType, types } from "../src";
 import { ClassModel, action, extend, register, view, volatile, volatileAction } from "../src/class-model";
 import { $identifier } from "../src/symbols";
+import { NameExample } from "./fixtures/NameExample";
 import { NamedThingClass, TestClassModel } from "./fixtures/TestClassModel";
 import { NamedThing, TestModelSnapshot } from "./fixtures/TestModel";
 import { create } from "./helpers";
-import { NameExample } from "./fixtures/NameExample";
 
 const DynamicNameExample = register(
   class extends ClassModel({ key: types.identifier, name: types.string }) {
@@ -146,6 +145,9 @@ class AutoIdentified extends ClassModel({ key: types.optional(types.identifier,
 @register
 class ParentOfMQT extends ClassModel({ key: types.identifier, thing: NamedThing }) {}
 
+@register
+class NestedComplex extends ClassModel({ stringArray: types.array(types.string), numberMap: types.map(types.number) }) {}
+
 const ParentOfModelClass = types.model("ParentOfModelClass", {
   key: types.identifier,
   child: NameExample,
@@ -513,6 +515,14 @@ describe("class models", () => {
     });
   });
 
+  describe("class models with array/map/complex properties", () => {
+    test("should default to empty arrays/maps when createReadOnly with undefined values", () => {
+      const instance = NestedComplex.createReadOnly({});
+      expect(Array.from(instance.stringArray)).toEqual([]);
+      expect(Object.fromEntries(Object.entries(instance.numberMap))).toEqual({});
+    });
+  });
+
   test("class model classes are IClassModelType", () => {
     const _testA: IClassModelType<{ key: ISimpleType<string>; name: ISimpleType<string> }> = NameExample;
     const _testB: IClassModelType<{ key: ISimpleType<string>; name: ISimpleType<string> }> = NamedThingClass;

diff --git a/src/array.ts b/src/array.ts
@@ -16,8 +16,8 @@ export class QuickArray<T extends IAnyType> extends Array<Instance<T>> implement
   /** @hidden */
   readonly [$type]: [this] | [any];
 
-  constructor(type: any, parent: IStateTreeNode | null, env: any) {
-    super();
+  constructor(type: any, parent: IStateTreeNode | null, env: any, ...items: Instance<T>[]) {
+    super(...items);
     this[$type] = type;
     this[$parent] = parent;
     this[$env] = env;

diff --git a/src/fast-instantiator.ts b/src/fast-instantiator.ts
@@ -29,7 +29,6 @@ const isDirectlyAssignableType = (type: IAnyType): type is DirectlyAssignableTyp
     type instanceof LiteralType ||
     type instanceof DateType ||
     type instanceof FrozenType ||
-    (type instanceof ArrayType && isDirectlyAssignableType(type.childrenType) && !(type.childrenType instanceof DateType)) ||
     type instanceof IntegerType
   );
 };
@@ -52,6 +51,8 @@ class InstantiatorBuilder<T extends IClassModelType<Record<string, IAnyType>, an
         segments.push(this.assignmentExpressionForOptionalType(key, type));
       } else if (type instanceof ReferenceType || type instanceof SafeReferenceType) {
         segments.push(this.assignmentExpressionForReferenceType(key, type));
+      } else if (type instanceof ArrayType) {
+        segments.push(this.assignmentExpressionForArrayType(key, type));
       } else if (type instanceof MapType) {
         segments.push(this.assignmentExpressionForMapType(key, type));
       } else {
@@ -183,24 +184,28 @@ class InstantiatorBuilder<T extends IClassModelType<Record<string, IAnyType>, an
     `;
   }
 
-  private assignmentExpressionForArrayType(key: string, _type: MapType<any>): string {
-    const mapVarName = `map${key}`;
-    const snapshotVarName = `snapshotValue${key}`;
+  private assignmentExpressionForArrayType(key: string, type: ArrayType<any>): string {
+    if (!isDirectlyAssignableType(type.childrenType) || type.childrenType instanceof DateType) {
+      return `
+        // instantiate fallback for ${key} of type ${type.name}
+        instance["${key}"] = ${this.alias(`model.properties["${key}"]`)}.instantiate(
+          snapshot?.["${key}"],
+          context,
+          instance
+        );
+      `;
+    }
+
+    // Directly assignable types are primitives so we don't need to worry about setting parent/env/etc. Hence, we just
+    // pass the snapshot straight through to the constructor.
     return `
-      const ${mapVarName} = new QuickArray(${this.alias(`model.properties["${key}"]`)}, instance, context.env);
-      instance["${key}"] = ${mapVarName};
-      const ${snapshotVarName} = snapshot?.["${key}"];
-      if (${snapshotVarName}) {
-        for (let index = 0; index < ${snapshotVarName}.length; ++index) {
-          ${mapVarName}.push(
-            ${this.alias(`model.properties["${key}"].childrenType`)}.instantiate(
-              ${snapshotVarName}[index],
-              context,
-              ${mapVarName}
-            )
-          );
-        }
-      }`;
+      instance["${key}"] = new QuickArray(
+        ${this.alias(`model.properties["${key}"]`)},
+        instance,
+        context.env,
+        ...(snapshot?.["${key}"] ?? [])
+      );
+    `;
   }
 
   private assignmentExpressionForMapType(key: string, _type: MapType<any>): string {