From 1fb7d794ada39324f4ce908dacbe2da9eb42c4ff Mon Sep 17 00:00:00 2001 From: Kyle Cox Date: Fri, 9 Oct 2026 02:41:36 +0000 Subject: [PATCH] fix(common): don't mutate the caller's array in Collection indexed adds _addOneOrMore reversed the input array in place when an index option was given, so a caller's array came back in reverse order (and a frozen array threw), even when the add itself failed. Insert by walking the array backwards instead, which keeps the inserted items in their original order at the target index without touching the input. --- .changeset/tidy-adds-keep.md | 5 +++ packages/common/src/models/collection.spec.ts | 33 +++++++++++++++++++ packages/common/src/models/collection.ts | 14 +++++--- 3 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 .changeset/tidy-adds-keep.md diff --git a/.changeset/tidy-adds-keep.md b/.changeset/tidy-adds-keep.md new file mode 100644 index 00000000..43efe7c0 --- /dev/null +++ b/.changeset/tidy-adds-keep.md @@ -0,0 +1,5 @@ +--- +"@fleet-sdk/common": patch +--- + +Fix `Collection.add()` mutating the caller's array when adding multiple items at a specific index: the items are now inserted in reverse order by index instead of reversing the input array in place, so frozen arrays work and a caller's array keeps its order even when the add fails. diff --git a/packages/common/src/models/collection.spec.ts b/packages/common/src/models/collection.spec.ts index 641610f7..ff7aa61f 100644 --- a/packages/common/src/models/collection.spec.ts +++ b/packages/common/src/models/collection.spec.ts @@ -81,6 +81,39 @@ describe("collection base", () => { expect(collection.at(4)).toBe(3); }); + it("Should not mutate the caller's array when placing multiple items at a specific index", () => { + const collection = new MockCollection(); + collection.add([1, 2, 3]); + + const items = [10, 20, 30]; + collection.add(items, { index: 1 }); + + expect(collection.toArray()).toEqual([1, 10, 20, 30, 2, 3]); + expect(items).toEqual([10, 20, 30]); + }); + + it("Should place a frozen array's items at a specific index", () => { + const collection = new MockCollection(); + collection.add([1, 2, 3]); + + collection.add(Object.freeze([10, 20]), { index: 2 }); + + expect(collection.toArray()).toEqual([1, 2, 10, 20, 3]); + }); + + it("Should not mutate the caller's array when an indexed add fails", () => { + const collection = new MockCollection(); + collection.add([1, 2, 3]); + + const items = [10, 20]; + expect(() => { + collection.add(items, { index: 5 }); + }).toThrow(RangeError); + + expect(items).toEqual([10, 20]); + expect(collection.toArray()).toEqual([1, 2, 3]); + }); + it("Should create a copy of the internal array", () => { const collection = new MockCollection(); collection.add(numbers); diff --git a/packages/common/src/models/collection.ts b/packages/common/src/models/collection.ts index 44562bc4..e028f7e4 100644 --- a/packages/common/src/models/collection.ts +++ b/packages/common/src/models/collection.ts @@ -110,11 +110,15 @@ export abstract class Collection implements Iterable protected _addOneOrMore(items: OneOrMore, options?: CollectionAddOptions): number { if (Array.isArray(items)) { if (isDefined(options) && isDefined(options.index)) { - items = items.reverse(); - } - - for (const item of items) { - this._addOne(item, options); + // Insert in reverse order so the items keep their original order at + // the target index, without mutating the caller's array. + for (let i = items.length - 1; i >= 0; i--) { + this._addOne(items[i], options); + } + } else { + for (const item of items) { + this._addOne(item, options); + } } } else { this._addOne(items, options);