Skip to content

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

Merged
chrishamm merged 1 commit into
Duet3D:v3.7-devfrom
jaysuk:fix/model-collection-species
Aug 12, 2026
Merged

Fix ModelCollection missing Symbol.species, crashing on .push() after filter/map/slice#1
chrishamm merged 1 commit into
Duet3D:v3.7-devfrom
jaysuk:fix/model-collection-species

Conversation

@jaysuk

@jaysuk jaysuk commented Aug 12, 2026

Copy link
Copy Markdown

Summary

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:

constructor(itemConstructor: { new(): T }, itemFactory: ItemFactory<T> | null = null) {
    super();
    Object.setPrototypeOf(this, ModelCollection.prototype);
    Object.defineProperty(this, "$itemConstructor", { enumerable: false, value: itemConstructor });
    ...
}

So a derived array ends up with $itemConstructor set to a number (the length). Any later .push() on it then throws:

TypeError: Right-hand side of 'instanceof' is not an object

— since push() checks item instanceof that.$itemConstructor.

This reliably crashes any consumer — DWC core or a plugin — that does something as ordinary as:

objectModel.tools.filter(t => isOffsettable(t)).push(someCommand)

Nothing catches a TypeError thrown deep inside a shared model getter/setter, so it takes down the whole page.

How I found this

Investigating a crash report in a DWC plugin (duet-tool-align) that builds a G-code command list via objectModel.tools.filter(isOffsettable).map(g10For).filter(Boolean), then does cmds.push(saveCommand). Traced the minified production stack trace back to this exact class/method in a locally-built copy of v3.7-dev and reproduced it directly against ModelCollection with no plugin-specific logic involved — any chain of derived-array operations ending in .push() reproduces it.

Minimal repro (also covered by the added test):

const collection = new ModelCollection(SomeItemClass);
collection.push(/* ...items... */);
const filtered = collection.filter(() => true); // silently becomes a ModelCollection with $itemConstructor = collection.length
filtered.push(newItem); // throws: Right-hand side of 'instanceof' is not an object

Fix

Override Symbol.species to return the plain Array constructor, so filter/map/slice/etc. return ordinary arrays. This leaves 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.

Testing

  • Added __tests__/collection.ts: constructs a ModelCollection, derives via filter/map, and asserts the result is a plain array and that .push() on it doesn't throw. Confirmed this test fails with the exact reported error on the unmodified class, and passes with the fix.
  • npx tsc -p tsconfig.test.json --noEmit — clean.
  • Full jest suite (npx jest --config jestconfig.json) — 36/36 passing, including the new test.

… 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.
@chrishamm
chrishamm merged commit e9e235b into Duet3D:v3.7-dev Aug 12, 2026
1 check passed
@chrishamm

Copy link
Copy Markdown
Contributor

Thanks! I may clean up the comment above static override get [Symbol.species](): ArrayConstructor though.

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants