From c5b0aa4edfaf5c5b2d085622cc0a6ba7eb30694b Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Fri, 11 Aug 2023 11:39:45 +0200 Subject: [PATCH 1/3] Avoid unwanted mutation in Group --- src/shapes/Group.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shapes/Group.ts b/src/shapes/Group.ts index 2f00e5fa54c..6bfac5f03c9 100644 --- a/src/shapes/Group.ts +++ b/src/shapes/Group.ts @@ -173,7 +173,7 @@ export class Group extends createCollectionMixin( objectsRelativeToGroup?: boolean ) { super(); - this._objects = objects; + this._objects = objects.slice(); // Avoid unwanted mutations of Collection to affect the caller this.__objectMonitor = this.__objectMonitor.bind(this); this.__objectSelectionTracker = this.__objectSelectionMonitor.bind( this, From 19d9149dc1c8f6f6af9a30b55b0ae9fa92b307b3 Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Fri, 11 Aug 2023 11:42:31 +0200 Subject: [PATCH 2/3] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3e005c5f77..a316d125205 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- patch(): Avoid unwanted mutation in Group [#9151](https://github.com/fabricjs/fabric.js/pull/9151) - patch(): ActiveSelection initialization + types [#9143](https://github.com/fabricjs/fabric.js/pull/9143) - chore(TS): BREAKING remove canvas.interactive, added typings for canvas options [#9140](https://github.com/fabricjs/fabric.js/pull/9140) - chore(TS): BREAKING PREVIOUS BETA mv + rename `TProps` => `TOptions` [#9139](https://github.com/fabricjs/fabric.js/pull/9139) From 2937325cd43475b1a928e5515023939dc67379cc Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Sun, 13 Aug 2023 22:03:12 +0200 Subject: [PATCH 3/3] Fix review comments --- CHANGELOG.md | 2 +- src/shapes/Group.spec.ts | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 src/shapes/Group.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a316d125205..188a8d5a05e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [next] -- patch(): Avoid unwanted mutation in Group [#9151](https://github.com/fabricjs/fabric.js/pull/9151) +- patch(): Avoid unwanted mutation to passed objects array to Group constructor [#9151](https://github.com/fabricjs/fabric.js/pull/9151) - patch(): ActiveSelection initialization + types [#9143](https://github.com/fabricjs/fabric.js/pull/9143) - chore(TS): BREAKING remove canvas.interactive, added typings for canvas options [#9140](https://github.com/fabricjs/fabric.js/pull/9140) - chore(TS): BREAKING PREVIOUS BETA mv + rename `TProps` => `TOptions` [#9139](https://github.com/fabricjs/fabric.js/pull/9139) diff --git a/src/shapes/Group.spec.ts b/src/shapes/Group.spec.ts new file mode 100644 index 00000000000..af57f825dd6 --- /dev/null +++ b/src/shapes/Group.spec.ts @@ -0,0 +1,15 @@ +import { Group } from './Group'; +import { FabricObject } from './Object/FabricObject'; + +describe('Group', () => { + it('avoid mutations to passed objects array', () => { + const objs = [new FabricObject(), new FabricObject()]; + const group = new Group(objs); + + group.add(new FabricObject()); + + expect(group._objects).not.toBe(objs); + expect(objs).toHaveLength(2); + expect(group._objects).toHaveLength(3); + }); +});