From 4a5969e30dfed665adac89a796afcab496d05cc3 Mon Sep 17 00:00:00 2001
From: Jay S <github@jayuk.org>
Date: Wed, 12 Aug 2026 08:48:03 +0100
Subject: [PATCH] fix: override Symbol.species on ModelCollection so derived
 arrays are plain Arrays

ModelCollection extends Array but never overrides the static Symbol.species
getter. Per the ES2015 species-construction protocol, that means any Array
method that derives a new array from a ModelCollection instance (filter,
map, slice, concat, etc.) constructs the result via `new ModelCollection(n)`
where n is the derived array's length -- the same call shape as the plain
Array(length) constructor.

ModelCollection'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`.

This reliably crashes any consumer -- DWC itself or a plugin -- that does
something as ordinary as `objectModel.tools.filter(...).push(...)`, and
takes down the whole page since nothing catches a TypeError thrown deep
inside a shared model getter/setter. Reported against a plugin hitting this
via `objectModel.tools.filter(isOffsettable).map(g10For).filter(Boolean)`
then `.push(saveCommand)` -- no plugin-specific logic involved, any chain of
derived-array operations ending in .push() reproduces it.

Overriding Symbol.species to return the plain Array constructor makes
filter/map/slice/etc. return ordinary arrays, leaving the custom
push()/update() behaviour only on genuine ModelCollection instances
(constructed directly with an item type), which is the only place it's
meaningful anyway.

Added a regression test that reproduces the crash pre-fix and passes post-fix.
---
 __tests__/collection.ts | 29 +++++++++++++++++++++++++++++
 src/ModelCollection.ts  | 16 ++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 __tests__/collection.ts

diff --git a/__tests__/collection.ts b/__tests__/collection.ts
new file mode 100644
index 0000000..9bbfa27
--- /dev/null
+++ b/__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);
+});
diff --git a/src/ModelCollection.ts b/src/ModelCollection.ts
index 28b0708..14f8da2 100644
--- a/src/ModelCollection.ts
+++ b/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