Revert "fix(core): fix ResizableDirective memory leak from orphaned document …" (#12201)

This reverts commit 1e40e5effd.
This commit is contained in:
Tomasz Gnyp
2026-08-27 16:13:41 +01:00
committed by GitHub
parent 91b5164ab2
commit 77c549830f
2 changed files with 44 additions and 60 deletions
@@ -16,7 +16,7 @@
*/ */
import { TestBed } from '@angular/core/testing'; import { TestBed } from '@angular/core/testing';
import { ElementRef, EnvironmentInjector, NgZone, Renderer2, createEnvironmentInjector, runInInjectionContext } from '@angular/core'; import { ElementRef, Injector, NgZone, Renderer2, runInInjectionContext } from '@angular/core';
import { ResizableDirective } from './resizable.directive'; import { ResizableDirective } from './resizable.directive';
describe('ResizableDirective', () => { describe('ResizableDirective', () => {
@@ -24,7 +24,6 @@ describe('ResizableDirective', () => {
let renderer: Renderer2; let renderer: Renderer2;
let element: ElementRef; let element: ElementRef;
let directive: ResizableDirective; let directive: ResizableDirective;
let testEnvInjector: EnvironmentInjector;
const scrollTop = 0; const scrollTop = 0;
const scrollLeft = 0; const scrollLeft = 0;
@@ -40,14 +39,8 @@ describe('ResizableDirective', () => {
scrollLeft scrollLeft
}; };
let unlistenSpies: jasmine.Spy[];
const rendererMock = { const rendererMock = {
listen: jasmine.createSpy('listen').and.callFake(() => { listen: jasmine.createSpy('listen'),
const spy = jasmine.createSpy(`unlisten-${unlistenSpies.length}`);
unlistenSpies.push(spy);
return spy;
}),
setStyle: jasmine.createSpy('setStyle') setStyle: jasmine.createSpy('setStyle')
}; };
@@ -60,10 +53,6 @@ describe('ResizableDirective', () => {
}; };
beforeEach(() => { beforeEach(() => {
unlistenSpies = [];
rendererMock.listen.calls.reset();
rendererMock.setStyle.calls.reset();
TestBed.configureTestingModule({ TestBed.configureTestingModule({
imports: [ResizableDirective], imports: [ResizableDirective],
providers: [ providers: [
@@ -75,28 +64,29 @@ describe('ResizableDirective', () => {
element = TestBed.inject(ElementRef); element = TestBed.inject(ElementRef);
renderer = TestBed.inject(Renderer2); renderer = TestBed.inject(Renderer2);
ngZone = TestBed.inject(NgZone); ngZone = TestBed.inject(NgZone);
const injector = TestBed.inject(Injector);
spyOn(ngZone, 'runOutsideAngular').and.callFake((fn) => fn()); spyOn(ngZone, 'runOutsideAngular').and.callFake((fn) => fn());
spyOn(ngZone, 'run').and.callFake((fn) => fn()); spyOn(ngZone, 'run').and.callFake((fn) => fn());
testEnvInjector = createEnvironmentInjector( const testInjector = Injector.create({
[ providers: [
{ provide: Renderer2, useValue: renderer }, { provide: Renderer2, useValue: renderer },
{ provide: ElementRef, useValue: element }, { provide: ElementRef, useValue: element },
{ provide: NgZone, useValue: ngZone } { provide: NgZone, useValue: ngZone }
], ],
TestBed.inject(EnvironmentInjector) parent: injector
); });
directive = runInInjectionContext(testEnvInjector, () => new ResizableDirective()); directive = runInInjectionContext(testInjector, () => new ResizableDirective());
directive.ngOnInit(); directive.ngOnInit();
}); });
it('should not attach any document listeners on init', () => { it('should attach mousedown event to document', () => {
expect(renderer.listen).not.toHaveBeenCalled(); expect(renderer.listen).toHaveBeenCalledWith('document', 'mousedown', jasmine.any(Function));
}); });
it('should attach document mousemove listener only during active drag', () => { it('should attach mousemove event to document', () => {
const mouseDownEvent = new MouseEvent('mousedown'); const mouseDownEvent = new MouseEvent('mousedown');
directive.mousedown.next({ ...mouseDownEvent, resize: true }); directive.mousedown.next({ ...mouseDownEvent, resize: true });
@@ -104,6 +94,10 @@ describe('ResizableDirective', () => {
expect(renderer.listen).toHaveBeenCalledWith('document', 'mousemove', jasmine.any(Function)); expect(renderer.listen).toHaveBeenCalledWith('document', 'mousemove', jasmine.any(Function));
}); });
it('should attach mouseup event to document', () => {
expect(renderer.listen).toHaveBeenCalledWith('document', 'mouseup', jasmine.any(Function));
});
it('should should set the cursor on mouse down', () => { it('should should set the cursor on mouse down', () => {
spyOn(directive.resizeStart, 'emit'); spyOn(directive.resizeStart, 'emit');
const mouseDownEvent = new MouseEvent('mousedown'); const mouseDownEvent = new MouseEvent('mousedown');
@@ -180,35 +174,4 @@ describe('ResizableDirective', () => {
expect(directive.keyboardResizing.emit).toHaveBeenCalledWith({ rectangle: { top: 0, left: 0, bottom: 0, right: step, width: step } }); expect(directive.keyboardResizing.emit).toHaveBeenCalledWith({ rectangle: { top: 0, left: 0, bottom: 0, right: step, width: step } });
}); });
it('should unregister document listeners on destroy', () => {
directive.mousedown.next({ ...new MouseEvent('mousedown'), resize: true });
expect(unlistenSpies.length).toBeGreaterThan(0);
testEnvInjector.destroy();
unlistenSpies.forEach((spy) => expect(spy).toHaveBeenCalledTimes(1));
});
it('should not accumulate mousemove listeners across repeated drag cycles', () => {
const listenCountAfterInit = rendererMock.listen.calls.count();
const mouseDownEvent = new MouseEvent('mousedown');
const mouseUpEvent = new MouseEvent('mouseup');
directive.mousedown.next({ ...mouseDownEvent, resize: true });
const listenCountAfterFirstDrag = rendererMock.listen.calls.count();
expect(listenCountAfterFirstDrag).toBeGreaterThan(listenCountAfterInit);
directive.mouseup.next(mouseUpEvent);
const unlistenedAfterFirstDrag = unlistenSpies.filter((spy) => spy.calls.count() > 0).length;
directive.mousedown.next({ ...mouseDownEvent, resize: true });
directive.mouseup.next(mouseUpEvent);
const unlistenedAfterSecondDrag = unlistenSpies.filter((spy) => spy.calls.count() > 0).length;
expect(unlistenedAfterSecondDrag).toBeGreaterThan(unlistenedAfterFirstDrag);
testEnvInjector.destroy();
unlistenSpies.forEach((spy) => expect(spy).toHaveBeenCalledTimes(1));
});
}); });
@@ -61,35 +61,53 @@ export class ResizableDirective implements OnInit, OnDestroy {
mousemove = new Subject<IResizeMouseEvent>(); mousemove = new Subject<IResizeMouseEvent>();
private readonly pointerDown: Observable<IResizeMouseEvent>;
private readonly pointerMove: Observable<IResizeMouseEvent>; private readonly pointerMove: Observable<IResizeMouseEvent>;
private readonly pointerUp: Observable<IResizeMouseEvent>;
private startingRect: BoundingRectangle; private startingRect: BoundingRectangle;
private currentRect: BoundingRectangle; private currentRect: BoundingRectangle;
private unsubscribeMouseDown?: () => void;
private unsubscribeMouseMove?: () => void;
private unsubscribeMouseUp?: () => void;
private readonly destroyRef = inject(DestroyRef); private readonly destroyRef = inject(DestroyRef);
constructor() { constructor() {
const renderer = this.renderer; const renderer = this.renderer;
const zone = this.zone; const zone = this.zone;
// Document-level mousemove is needed for smooth drag tracking when cursor leaves the handle element. this.pointerDown = new Observable((observer: Observer<IResizeMouseEvent>) => {
// Only subscribed during active drag via share() refcount.
this.pointerMove = new Observable((observer: Observer<IResizeMouseEvent>) => {
let stopListening: () => void = () => {};
zone.runOutsideAngular(() => { zone.runOutsideAngular(() => {
stopListening = renderer.listen('document', 'mousemove', (event: MouseEvent) => { this.unsubscribeMouseDown = renderer.listen('document', 'mousedown', (event: MouseEvent) => {
observer.next(event);
});
});
}).pipe(share());
this.pointerMove = new Observable((observer: Observer<IResizeMouseEvent>) => {
zone.runOutsideAngular(() => {
this.unsubscribeMouseMove = renderer.listen('document', 'mousemove', (event: MouseEvent) => {
observer.next(event);
});
});
}).pipe(share());
this.pointerUp = new Observable((observer: Observer<IResizeMouseEvent>) => {
zone.runOutsideAngular(() => {
this.unsubscribeMouseUp = renderer.listen('document', 'mouseup', (event: MouseEvent) => {
observer.next(event); observer.next(event);
}); });
}); });
return stopListening;
}).pipe(share()); }).pipe(share());
} }
ngOnInit(): void { ngOnInit(): void {
const mousedown$ = this.mousedown.asObservable(); const mousedown$ = merge(this.pointerDown, this.mousedown);
const mousemove$ = merge(this.pointerMove, this.mousemove); const mousemove$ = merge(this.pointerMove, this.mousemove);
const mouseup$ = this.mouseup.asObservable(); const mouseup$ = merge(this.pointerUp, this.mouseup);
const mouseDrag: Observable<IResizeMouseEvent | ICoordinateX> = mousedown$ const mouseDrag: Observable<IResizeMouseEvent | ICoordinateX> = mousedown$
.pipe( .pipe(
@@ -166,6 +184,9 @@ export class ResizableDirective implements OnInit, OnDestroy {
this.mousedown.complete(); this.mousedown.complete();
this.mousemove.complete(); this.mousemove.complete();
this.mouseup.complete(); this.mouseup.complete();
this.unsubscribeMouseDown?.();
this.unsubscribeMouseMove?.();
this.unsubscribeMouseUp?.();
} }
resizeByKeyboard(delta: number): void { resizeByKeyboard(delta: number): void {