From 77c549830fef1ca2c114fa600f49e1083ace8370 Mon Sep 17 00:00:00 2001 From: Tomasz Gnyp <49343696+tomgny@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:13:41 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"fix(core):=20fix=20ResizableDirective?= =?UTF-8?q?=20memory=20leak=20from=20orphaned=20document=20=E2=80=A6"=20(#?= =?UTF-8?q?12201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 1e40e5effd96c0ea77e886a792a67ca3c7fb955a. --- .../resizable/resizable.directive.spec.ts | 67 +++++-------------- .../resizable/resizable.directive.ts | 37 +++++++--- 2 files changed, 44 insertions(+), 60 deletions(-) diff --git a/lib/core/src/lib/datatable/directives/resizable/resizable.directive.spec.ts b/lib/core/src/lib/datatable/directives/resizable/resizable.directive.spec.ts index 0e0b19140b..3317dfeeb2 100644 --- a/lib/core/src/lib/datatable/directives/resizable/resizable.directive.spec.ts +++ b/lib/core/src/lib/datatable/directives/resizable/resizable.directive.spec.ts @@ -16,7 +16,7 @@ */ 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'; describe('ResizableDirective', () => { @@ -24,7 +24,6 @@ describe('ResizableDirective', () => { let renderer: Renderer2; let element: ElementRef; let directive: ResizableDirective; - let testEnvInjector: EnvironmentInjector; const scrollTop = 0; const scrollLeft = 0; @@ -40,14 +39,8 @@ describe('ResizableDirective', () => { scrollLeft }; - let unlistenSpies: jasmine.Spy[]; - const rendererMock = { - listen: jasmine.createSpy('listen').and.callFake(() => { - const spy = jasmine.createSpy(`unlisten-${unlistenSpies.length}`); - unlistenSpies.push(spy); - return spy; - }), + listen: jasmine.createSpy('listen'), setStyle: jasmine.createSpy('setStyle') }; @@ -60,10 +53,6 @@ describe('ResizableDirective', () => { }; beforeEach(() => { - unlistenSpies = []; - rendererMock.listen.calls.reset(); - rendererMock.setStyle.calls.reset(); - TestBed.configureTestingModule({ imports: [ResizableDirective], providers: [ @@ -75,28 +64,29 @@ describe('ResizableDirective', () => { element = TestBed.inject(ElementRef); renderer = TestBed.inject(Renderer2); ngZone = TestBed.inject(NgZone); + const injector = TestBed.inject(Injector); spyOn(ngZone, 'runOutsideAngular').and.callFake((fn) => fn()); spyOn(ngZone, 'run').and.callFake((fn) => fn()); - testEnvInjector = createEnvironmentInjector( - [ + const testInjector = Injector.create({ + providers: [ { provide: Renderer2, useValue: renderer }, { provide: ElementRef, useValue: element }, { provide: NgZone, useValue: ngZone } ], - TestBed.inject(EnvironmentInjector) - ); + parent: injector + }); - directive = runInInjectionContext(testEnvInjector, () => new ResizableDirective()); + directive = runInInjectionContext(testInjector, () => new ResizableDirective()); directive.ngOnInit(); }); - it('should not attach any document listeners on init', () => { - expect(renderer.listen).not.toHaveBeenCalled(); + it('should attach mousedown event to document', () => { + 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'); directive.mousedown.next({ ...mouseDownEvent, resize: true }); @@ -104,6 +94,10 @@ describe('ResizableDirective', () => { 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', () => { spyOn(directive.resizeStart, 'emit'); 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 } }); }); - - 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)); - }); }); diff --git a/lib/core/src/lib/datatable/directives/resizable/resizable.directive.ts b/lib/core/src/lib/datatable/directives/resizable/resizable.directive.ts index 9b7436759c..06d168a006 100644 --- a/lib/core/src/lib/datatable/directives/resizable/resizable.directive.ts +++ b/lib/core/src/lib/datatable/directives/resizable/resizable.directive.ts @@ -61,35 +61,53 @@ export class ResizableDirective implements OnInit, OnDestroy { mousemove = new Subject(); + private readonly pointerDown: Observable; private readonly pointerMove: Observable; + private readonly pointerUp: Observable; private startingRect: BoundingRectangle; private currentRect: BoundingRectangle; + private unsubscribeMouseDown?: () => void; + private unsubscribeMouseMove?: () => void; + private unsubscribeMouseUp?: () => void; + private readonly destroyRef = inject(DestroyRef); constructor() { const renderer = this.renderer; const zone = this.zone; - // Document-level mousemove is needed for smooth drag tracking when cursor leaves the handle element. - // Only subscribed during active drag via share() refcount. - this.pointerMove = new Observable((observer: Observer) => { - let stopListening: () => void = () => {}; + this.pointerDown = new Observable((observer: Observer) => { 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) => { + zone.runOutsideAngular(() => { + this.unsubscribeMouseMove = renderer.listen('document', 'mousemove', (event: MouseEvent) => { + observer.next(event); + }); + }); + }).pipe(share()); + + this.pointerUp = new Observable((observer: Observer) => { + zone.runOutsideAngular(() => { + this.unsubscribeMouseUp = renderer.listen('document', 'mouseup', (event: MouseEvent) => { observer.next(event); }); }); - return stopListening; }).pipe(share()); } ngOnInit(): void { - const mousedown$ = this.mousedown.asObservable(); + const mousedown$ = merge(this.pointerDown, this.mousedown); const mousemove$ = merge(this.pointerMove, this.mousemove); - const mouseup$ = this.mouseup.asObservable(); + const mouseup$ = merge(this.pointerUp, this.mouseup); const mouseDrag: Observable = mousedown$ .pipe( @@ -166,6 +184,9 @@ export class ResizableDirective implements OnInit, OnDestroy { this.mousedown.complete(); this.mousemove.complete(); this.mouseup.complete(); + this.unsubscribeMouseDown?.(); + this.unsubscribeMouseMove?.(); + this.unsubscribeMouseUp?.(); } resizeByKeyboard(delta: number): void {