revert(): rm active selection ref

.codesandbox/templates/next/components/Canvas.tsx

@@ -28,6 +28,8 @@ export const Canvas = React.forwardRef<
       ref.current = canvas;
     }
 
+    // it is crucial `onLoad` is a dependency of this effect
+    // to ensure the canvas is disposed and re-created if it changes
     onLoad?.(canvas);
 
     return () => {

CHANGELOG.md

@@ -15,6 +15,7 @@
 - feat(): Add save/restore ability to group LayoutManager [#9564](https://github.com/fabricjs/fabric.js/pull/9564)
 - fix(): Remove unwanted set type warning [#9569](https://github.com/fabricjs/fabric.js/pull/9569)
 - refactor(): Separate defaults for base fabric object vs interactive object. Also some moving around of variables [#9474](https://github.com/fabricjs/fabric.js/pull/9474)
+- revert(): rm active selection ref [#9561](https://github.com/fabricjs/fabric.js/pull/9561)
 - refactor(): Change how LayoutManager handles restoring groups [#9522](https://github.com/fabricjs/fabric.js/pull/9522)
 - fix(BaseConfiguration): set `devicePixelRatio` from window [#9470](https://github.com/fabricjs/fabric.js/pull/9470)
 - fix(): bubble dirty flag for group only when true [#9540](https://github.com/fabricjs/fabric.js/pull/9540)

src/ClassRegistry.ts

@@ -16,15 +16,15 @@
 export const SVG = 'svg';
 
 export class ClassRegistry {
   declare [JSON]: Map<string, any>;
   declare [SVG]: Map<string, any>;
 
   constructor() {
     this[JSON] = new Map();
     this[SVG] = new Map();
   }
 
-  getClass(classType: string): any {
+  getClass<T>(classType: string): T {
     const constructor = this[JSON].get(classType);
     if (!constructor) {
       throw new FabricError(`No class registered for ${classType}`);
@@ -32,7 +32,7 @@
     return constructor;
   }
 
   setClass(classConstructor: any, classType?: string) {
     if (classType) {
       this[JSON].set(classType, classConstructor);
     } else {
@@ -43,11 +43,11 @@
     }
   }
 
   getSVGClass(SVGTagName: string): any {
     return this[SVG].get(SVGTagName);
   }
 
   setSVGClass(classConstructor: any, SVGTagName?: string) {
     this[SVG].set(
       SVGTagName ?? classConstructor.type.toLowerCase(),
       classConstructor

src/canvas/Canvas.ts

@@ -1,3 +1,4 @@
+import { classRegistry } from '../ClassRegistry';
 import { NONE } from '../constants';
 import type {
   CanvasEvents,
@@ -8,12 +9,14 @@ import type {
   Transform,
 } from '../EventTypeDefs';
 import { Point } from '../Point';
+import type { ActiveSelection } from '../shapes/ActiveSelection';
 import type { Group } from '../shapes/Group';
 import type { IText } from '../shapes/IText/IText';
 import type { FabricObject } from '../shapes/Object/FabricObject';
 import { isTouchEvent, stopEvent } from '../util/dom_event';
 import { getDocumentFromElement, getWindowFromElement } from '../util/dom_misc';
 import { sendPointToPlane } from '../util/misc/planeChange';
+import { isActiveSelection } from '../util/typeAssertions';
 import type { CanvasOptions, TCanvasOptions } from './CanvasOptions';
 import { SelectableCanvas } from './SelectableCanvas';
 import { TextEditingManager } from './TextEditingManager';
@@ -1387,10 +1390,9 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
       return;
     }
     let hoverCursor = target.hoverCursor || this.hoverCursor;
-    const activeSelection =
-        this._activeObject === this._activeSelection
-          ? this._activeObject
-          : null,
+    const activeSelection = isActiveSelection(this._activeObject)
+        ? this._activeObject
+        : null,
       // only show proper corner when group selection is not active
       corner =
         (!activeSelection || target.group !== activeSelection) &&
@@ -1431,8 +1433,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
    */
   protected handleMultiSelection(e: TPointerEvent, target?: FabricObject) {
     const activeObject = this._activeObject;
-    const activeSelection = this._activeSelection;
-    const isAS = activeObject === activeSelection;
+    const isAS = isActiveSelection(activeObject);
     if (
       // check if an active object exists on canvas and if the user is pressing the `selectionKey` while canvas supports multi selection.
       !!activeObject &&
@@ -1455,9 +1456,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
       !activeObject.getActiveControl()
     ) {
       if (isAS) {
-        const prevActiveObjects =
-          activeSelection.getObjects() as FabricObject[];
-        if (target === activeSelection) {
+        const prevActiveObjects = activeObject.getObjects();
+        if (target === activeObject) {
           const pointer = this.getViewportPoint(e);
           target =
             // first search active objects for a target to remove
@@ -1470,32 +1470,42 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
             return false;
           }
         }
-        if (target.group === activeSelection) {
+        if (target.group === activeObject) {
           // `target` is part of active selection => remove it
-          activeSelection.remove(target);
+          activeObject.remove(target);
           this._hoveredTarget = target;
           this._hoveredTargets = [...this.targets];
-          if (activeSelection.size() === 1) {
+          if (activeObject.size() === 1) {
             // activate last remaining object
-            this._setActiveObject(activeSelection.item(0) as FabricObject, e);
+            // deselecting the active selection will remove the remaining object from it
+            this._setActiveObject(activeObject.item(0), e);
           }
         } else {
           //  `target` isn't part of active selection => add it
-          activeSelection.multiSelectAdd(target);
-          this._hoveredTarget = activeSelection;
+          activeObject.multiSelectAdd(target);
+          this._hoveredTarget = activeObject;
           this._hoveredTargets = [...this.targets];
         }
         this._fireSelectionEvents(prevActiveObjects, e);
       } else {
         (activeObject as IText).exitEditing &&
           (activeObject as IText).exitEditing();
         // add the active object and the target to the active selection and set it as the active object
-        activeSelection.multiSelectAdd(activeObject, target);
-        this._hoveredTarget = activeSelection;
+        const klass =
+          classRegistry.getClass<typeof ActiveSelection>('ActiveSelection');
+        const newActiveSelection = new klass([], {
+          /**
+           * it is crucial to pass the canvas ref before calling {@link ActiveSelection#multiSelectAdd}
+           * since it uses {@link FabricObject#isInFrontOf} which relies on the canvas ref
+           */
+          canvas: this,
+        });
+        newActiveSelection.multiSelectAdd(activeObject, target);
+        this._hoveredTarget = newActiveSelection;
         // ISSUE 4115: should we consider subTargets here?
         // this._hoveredTargets = [];
         // this._hoveredTargets = this.targets.concat();
-        this._setActiveObject(activeSelection, e);
+        this._setActiveObject(newActiveSelection, e);
         this._fireSelectionEvents([activeObject], e);
       }
       return true;
@@ -1549,8 +1559,9 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
       this.setActiveObject(objects[0], e);
     } else if (objects.length > 1) {
       // add to active selection and make it the active object
-      this._activeSelection.add(...objects);
-      this.setActiveObject(this._activeSelection, e);
+      const klass =
+        classRegistry.getClass<typeof ActiveSelection>('ActiveSelection');
+      this.setActiveObject(new klass(objects, { canvas: this }), e);
     }
 
     // cleanup

src/canvas/CanvasOptions.ts

@@ -1,5 +1,4 @@
 import type { ModifierKey, TOptionalModifierKey } from '../EventTypeDefs';
-import type { ActiveSelection } from '../shapes/ActiveSelection';
 import type { TOptions } from '../typedefs';
 import type { StaticCanvasOptions } from './StaticCanvasOptions';
 
@@ -260,9 +259,7 @@ export interface CanvasOptions
   preserveObjectStacking: boolean;
 }
 
-export type TCanvasOptions = TOptions<
-  CanvasOptions & { activeSelection: ActiveSelection }
->;
+export type TCanvasOptions = TOptions<CanvasOptions>;
 
 export const canvasDefaults: TOptions<CanvasOptions> = {
   uniformScaling: true,

src/canvas/SelectableCanvas.ts

@@ -11,7 +11,6 @@ import type {
 } from '../EventTypeDefs';
 import {
   addTransformToObject,
-  resetObjectTransform,
   saveObjectTransform,
 } from '../util/misc/objectTransforms';
 import type { TCanvasSizeOptions } from './StaticCanvas';
@@ -31,13 +30,13 @@ import type { IText } from '../shapes/IText/IText';
 import type { BaseBrush } from '../brushes/BaseBrush';
 import { pick } from '../util/misc/pick';
 import { sendPointToPlane } from '../util/misc/planeChange';
-import { ActiveSelection } from '../shapes/ActiveSelection';
 import { cos, createCanvasElement, sin } from '../util';
 import { CanvasDOMManager } from './DOMManagers/CanvasDOMManager';
 import { BOTTOM, CENTER, LEFT, RIGHT, TOP } from '../constants';
-import type { CanvasOptions, TCanvasOptions } from './CanvasOptions';
+import type { CanvasOptions } from './CanvasOptions';
 import { canvasDefaults } from './CanvasOptions';
 import { Intersection } from '../Intersection';
+import { isActiveSelection } from '../util/typeAssertions';
 
 /**
  * Canvas class
@@ -294,17 +293,6 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   protected declare _isCurrentlyDrawing: boolean;
   declare freeDrawingBrush?: BaseBrush;
   declare _activeObject?: FabricObject;
-  protected _activeSelection: ActiveSelection;
-
-  constructor(
-    el?: string | HTMLCanvasElement,
-    { activeSelection = new ActiveSelection(), ...options }: TCanvasOptions = {}
-  ) {
-    super(el, options);
-    this._activeSelection = activeSelection;
-    this._activeSelection.set('canvas', this);
-    this._activeSelection.setCoords();
-  }
 
   protected initElements(el?: string | HTMLCanvasElement) {
     this.elements = new CanvasDOMManager(el, {
@@ -1029,27 +1017,17 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
     return this._activeObject;
   }
 
-  /**
-   * Returns instance's active selection
-   */
-  getActiveSelection() {
-    return this._activeSelection;
-  }
-
   /**
    * Returns an array with the current selected objects
    * @return {FabricObject[]} active objects array
    */
   getActiveObjects(): FabricObject[] {
     const active = this._activeObject;
-    if (active) {
-      if (active === this._activeSelection) {
-        return [...(active as ActiveSelection)._objects];
-      } else {
-        return [active];
-      }
-    }
-    return [];
+    return isActiveSelection(active)
+      ? active.getObjects()
+      : active
+      ? [active]
+      : [];
   }
 
   /**
@@ -1134,6 +1112,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
    * @return {Boolean} true if the object has been selected
    */
   _setActiveObject(object: FabricObject, e?: TPointerEvent) {
+    const prevActiveObject = this._activeObject;
     if (this._activeObject === object) {
       return false;
     }
@@ -1144,10 +1123,10 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
     if (object.onSelect({ e })) {
       return false;
     }
+
     this._activeObject = object;
 
-    if (object instanceof ActiveSelection && this._activeSelection !== object) {
-      this._activeSelection = object;
+    if (isActiveSelection(object) && prevActiveObject !== object) {
       object.set('canvas', this);
       object.setCoords();
     }
@@ -1173,11 +1152,6 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
       if (obj.onDeselect({ e, object })) {
         return false;
       }
-      // clear active selection
-      if (obj === this._activeSelection) {
-        this._activeSelection.removeAll();
-        resetObjectTransform(this._activeSelection);
-      }
       if (this._currentTransform && this._currentTransform.target === obj) {
         // @ts-expect-error this method exists in the subclass - should be moved or declared as abstract
         this.endCurrentTransform(e);
@@ -1227,11 +1201,13 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
    */
   destroy() {
     // dispose of active selection
-    const activeSelection = this._activeSelection;
-    activeSelection.removeAll();
-    // @ts-expect-error disposing
-    this._activeSelection = undefined;
-    activeSelection.dispose();
+    const activeObject = this._activeObject;
+    if (isActiveSelection(activeObject)) {
+      activeObject.removeAll();
+      activeObject.dispose();
+    }
+
+    delete this._activeObject;
 
     super.destroy();
 
@@ -1296,11 +1272,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   _realizeGroupTransformOnObject(
     instance: FabricObject
   ): Partial<typeof instance> {
-    if (
-      instance.group &&
-      instance.group === this._activeSelection &&
-      this._activeObject === instance.group
-    ) {
+    if (instance.group && isActiveSelection(instance.group)) {
       const layoutProps = [
         'angle',
         'flipX',
@@ -1313,7 +1285,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
         TOP,
       ] as (keyof typeof instance)[];
       const originalValues = pick<typeof instance>(instance, layoutProps);
-      addTransformToObject(instance, this._activeObject.calcOwnMatrix());
+      addTransformToObject(instance, instance.group.calcOwnMatrix());
       return originalValues;
     } else {
       return {};

src/filters/Composed.ts

@@ -67,7 +67,9 @@ export class Composed extends BaseFilter {
   ) {
     return Promise.all(
       ((object.subFilters || []) as BaseFilter[]).map((filter) =>
-        classRegistry.getClass(filter.type).fromObject(filter, options)
+        classRegistry
+          .getClass<typeof BaseFilter>(filter.type)
+          .fromObject(filter, options)
       )
     ).then(
       (enlivedFilters) => new this({ subFilters: enlivedFilters }) as BaseFilter

src/shapes/ActiveSelection.spec.ts

@@ -81,21 +81,6 @@ describe('ActiveSelection', () => {
     expect(spy).not.toHaveBeenCalled();
   });
 
-  it('sets coords after attaching to canvas', () => {
-    const canvas = new Canvas(undefined, {
-      activeSelection: new ActiveSelection([
-        new FabricObject({
-          left: 100,
-          top: 100,
-          width: 100,
-          height: 100,
-        }),
-      ]),
-      viewportTransform: [2, 0, 0, 0.5, 400, 150],
-    });
-    expect(canvas.getActiveSelection().aCoords).toMatchSnapshot();
-  });
-
   it('`setActiveObject` should update the active selection ref on canvas if it changed', () => {
     const canvas = new Canvas();
     const obj1 = new FabricObject();
@@ -104,7 +89,7 @@ describe('ActiveSelection', () => {
     const activeSelection = new ActiveSelection([obj1, obj2]);
     const spy = jest.spyOn(activeSelection, 'setCoords');
     canvas.setActiveObject(activeSelection);
-    expect(canvas.getActiveSelection()).toBe(activeSelection);
+    expect(canvas.getActiveObject()).toBe(activeSelection);
     expect(canvas.getActiveObjects()).toEqual([obj1, obj2]);
     expect(spy).toHaveBeenCalled();
     expect(activeSelection.canvas).toBe(canvas);
@@ -117,8 +102,7 @@ describe('ActiveSelection', () => {
   test('adding and removing an object belonging to a group', () => {
     const object = new FabricObject();
     const group = new Group([object]);
-    const canvas = new Canvas();
-    const activeSelection = canvas.getActiveSelection();
+    const activeSelection = new ActiveSelection();
 
     const eventsSpy = jest.spyOn(object, 'fire');
     const removeSpy = jest.spyOn(group, 'remove');
@@ -132,7 +116,6 @@ describe('ActiveSelection', () => {
     activeSelection.add(object);
     expect(object.group).toBe(activeSelection);
     expect(object.parent).toBe(group);
-    expect(object.canvas).toBe(canvas);
     expect(removeSpy).not.toBeCalled();
     expect(exitSpy).toBeCalledWith(object);
     expect(enterSpy).toBeCalledWith(object, true);
@@ -146,7 +129,6 @@ describe('ActiveSelection', () => {
     });
     expect(object.group).toBe(group);
     expect(object.parent).toBe(group);
-    expect(object.canvas).toBeUndefined();
   });
 
   test('transferring an object between active selections', () => {