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);