Merge pull request #1 from jaysuk/fix/model-collection-species

Fix ModelCollection missing Symbol.species, crashing on .push() after filter/map/slice

Author: chrishamm

__tests__/collection.ts

@@ -0,0 +1,29 @@
+import { ModelCollection } from "../src";
+import type { IModelObject } from "../src";
+
+class Item implements IModelObject {
+    value: number = 0;
+    update(jsonElement: any): IModelObject | null {
+        this.value = jsonElement?.value ?? 0;
+        return this;
+    }
+}
+
+test("array methods derived from a ModelCollection return plain arrays", () => {
+    const collection = new ModelCollection(Item);
+    collection.push(new Item().update({ value: 1 }) as Item, new Item().update({ value: 2 }) as Item);
+
+    const filtered = collection.filter(() => true);
+    expect(filtered).not.toBeInstanceOf(ModelCollection);
+
+    const mapped = collection.map((item) => item);
+    expect(mapped).not.toBeInstanceOf(ModelCollection);
+
+    // Regression: before overriding Symbol.species, `filtered`/`mapped` were still ModelCollection
+    // instances whose $itemConstructor had been corrupted to a number (the array length) by the
+    // default ES2015 species-construction protocol (`new ModelCollection(length)`). Pushing onto
+    // them then threw "Right-hand side of 'instanceof' is not an object" instead of behaving like
+    // a normal array.
+    expect(() => filtered.push(new Item().update({ value: 3 }) as Item)).not.toThrow();
+    expect(filtered.length).toBe(3);
+});

src/ModelCollection.ts

@@ -30,6 +30,22 @@ function createItem<T>(collection: IModelCollection<T>, index: number): T {
  * Class for storing model object items in an array
  */
 export class ModelCollection<T extends IModelObject | null> extends Array<T> implements IModelObject {
+    /**
+     * Without this, Array methods that derive a new array from this one (filter, map, slice, concat,
+     * etc.) build the result via the ES2015 species-construction protocol, which for an Array
+     * subclass means calling `new ModelCollection(length)` -- a single numeric argument, matching
+     * the plain Array(length) constructor signature. This class's own constructor instead treats
+     * that first argument as `itemConstructor`, so the derived array ends up with $itemConstructor
+     * set to a number. Any later `.push()` on it then throws "Right-hand side of 'instanceof' is not
+     * an object", since push() checks `item instanceof that.$itemConstructor`. Overriding the species
+     * to plain Array sidesteps this entirely: derived arrays are ordinary Arrays, and only genuine
+     * ModelCollection instances (constructed directly with an item type) get the custom push/update
+     * behaviour.
+     */
+    static override get [Symbol.species](): ArrayConstructor {
+        return Array;
+    }
+
     /**
      * Constructor of this class
      * @param itemConstructor Item constructor type that items must derive from