From 6fe84f317e82e8aa1e39516bc715e67c14928859 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 7 Nov 2018 18:10:05 +0000 Subject: [PATCH] fix(drag-drop): error if drag item is destroyed zone has stabilized (#13978) We initialize the `rootElement` in an `NgZone.onStable` callback, however if the zone doesn't stabilize before the `cdkDrag` is destroyed, we'll throw an error because there is no `rootElement`. These changes add an extra check to ensure that we don't throw. --- src/cdk/drag-drop/drag.spec.ts | 8 ++++++ src/cdk/drag-drop/drag.ts | 45 ++++++++++++++++++++-------------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/cdk/drag-drop/drag.spec.ts b/src/cdk/drag-drop/drag.spec.ts index b0c66f523b0..59c12654ace 100644 --- a/src/cdk/drag-drop/drag.spec.ts +++ b/src/cdk/drag-drop/drag.spec.ts @@ -427,6 +427,14 @@ describe('CdkDrag', () => { expect(event.stopPropagation).toHaveBeenCalled(); })); + it('should not throw if destroyed before the first change detection run', fakeAsync(() => { + const fixture = createComponent(StandaloneDraggable); + + expect(() => { + fixture.destroy(); + }).not.toThrow(); + })); + }); describe('draggable with a handle', () => { diff --git a/src/cdk/drag-drop/drag.ts b/src/cdk/drag-drop/drag.ts index 34942f83d2d..7085a60d9b6 100644 --- a/src/cdk/drag-drop/drag.ts +++ b/src/cdk/drag-drop/drag.ts @@ -175,6 +175,9 @@ export class CdkDrag implements AfterViewInit, OnDestroy { /** Subscription to the event that is dispatched when the user lifts their pointer. */ private _pointerUpSubscription = Subscription.EMPTY; + /** Subscription to the stream that initializes the root element. */ + private _rootElementInitSubscription = Subscription.EMPTY; + /** Elements that can be used to drag the draggable item. */ @ContentChildren(CdkDragHandle, {descendants: true}) _handles: QueryList; @@ -265,30 +268,36 @@ export class CdkDrag implements AfterViewInit, OnDestroy { // element to be in the proper place in the DOM. This is mostly relevant // for draggable elements inside portals since they get stamped out in // their original DOM position and then they get transferred to the portal. - this._ngZone.onStable.asObservable().pipe(take(1)).subscribe(() => { - const rootElement = this._rootElement = this._getRootElement(); - rootElement.addEventListener('mousedown', this._pointerDown, passiveEventListenerOptions); - rootElement.addEventListener('touchstart', this._pointerDown, passiveEventListenerOptions); - toggleNativeDragInteractions(rootElement , false); - }); + this._rootElementInitSubscription = this._ngZone.onStable.asObservable() + .pipe(take(1)) + .subscribe(() => { + const rootElement = this._rootElement = this._getRootElement(); + rootElement.addEventListener('mousedown', this._pointerDown, passiveEventListenerOptions); + rootElement.addEventListener('touchstart', this._pointerDown, passiveEventListenerOptions); + toggleNativeDragInteractions(rootElement , false); + }); } ngOnDestroy() { - this._rootElement.removeEventListener('mousedown', this._pointerDown, - passiveEventListenerOptions); - this._rootElement.removeEventListener('touchstart', this._pointerDown, - passiveEventListenerOptions); - this._destroyPreview(); - this._destroyPlaceholder(); + // The directive might have been destroyed before the root element is initialized. + if (this._rootElement) { + this._rootElement.removeEventListener('mousedown', this._pointerDown, + passiveEventListenerOptions); + this._rootElement.removeEventListener('touchstart', this._pointerDown, + passiveEventListenerOptions); - // Do this check before removing from the registry since it'll - // stop being considered as dragged once it is removed. - if (this._isDragging()) { - // Since we move out the element to the end of the body while it's being - // dragged, we have to make sure that it's removed if it gets destroyed. - this._removeElement(this._rootElement); + // Do this check before removing from the registry since it'll + // stop being considered as dragged once it is removed. + if (this._isDragging()) { + // Since we move out the element to the end of the body while it's being + // dragged, we have to make sure that it's removed if it gets destroyed. + this._removeElement(this._rootElement); + } } + this._rootElementInitSubscription.unsubscribe(); + this._destroyPreview(); + this._destroyPlaceholder(); this._nextSibling = null; this._dragDropRegistry.removeDragItem(this); this._removeSubscriptions();