Skip to content

Commit 7d29bc3

Browse files
crisbetozarend
authored andcommitted
perf(cdk/drag-drop): avoid excessive change detections with zone-patch-rxjs (#23272)
The `CdkDrag` directive subscribes to `NgZone.onStable` on init and then unsubscribes after the first emit. This is usually fine, but it can cause change detections if the app is using `zone-patch-rxjs`. These changes explicitly run a few sensitive calls outside the zone. Fixes #23248. (cherry picked from commit 0baca18)
1 parent 8d4f4e2 commit 7d29bc3

File tree

1 file changed

+59
-47
lines changed
  • src/cdk/drag-drop/directives

1 file changed

+59
-47
lines changed

src/cdk/drag-drop/directives/drag.ts

Lines changed: 59 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -274,51 +274,24 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
274274
}
275275

276276
ngAfterViewInit() {
277-
// We need to wait for the zone to stabilize, in order for the reference
278-
// element to be in the proper place in the DOM. This is mostly relevant
279-
// for draggable elements inside portals since they get stamped out in
280-
// their original DOM position and then they get transferred to the portal.
281-
this._ngZone.onStable
282-
.pipe(take(1), takeUntil(this._destroyed))
283-
.subscribe(() => {
284-
this._updateRootElement();
285-
286-
// Listen for any newly-added handles.
287-
this._handles.changes.pipe(
288-
startWith(this._handles),
289-
// Sync the new handles with the DragRef.
290-
tap((handles: QueryList<CdkDragHandle>) => {
291-
const childHandleElements = handles
292-
.filter(handle => handle._parentDrag === this)
293-
.map(handle => handle.element);
294-
295-
// Usually handles are only allowed to be a descendant of the drag element, but if
296-
// the consumer defined a different drag root, we should allow the drag element
297-
// itself to be a handle too.
298-
if (this._selfHandle && this.rootElementSelector) {
299-
childHandleElements.push(this.element);
300-
}
301-
302-
this._dragRef.withHandles(childHandleElements);
303-
}),
304-
// Listen if the state of any of the handles changes.
305-
switchMap((handles: QueryList<CdkDragHandle>) => {
306-
return merge(...handles.map(item => {
307-
return item._stateChanges.pipe(startWith(item));
308-
})) as Observable<CdkDragHandle>;
309-
}),
310-
takeUntil(this._destroyed)
311-
).subscribe(handleInstance => {
312-
// Enabled/disable the handle that changed in the DragRef.
313-
const dragRef = this._dragRef;
314-
const handle = handleInstance.element.nativeElement;
315-
handleInstance.disabled ? dragRef.disableHandle(handle) : dragRef.enableHandle(handle);
277+
// Normally this isn't in the zone, but it can cause major performance regressions for apps
278+
// using `zone-patch-rxjs` because it'll trigger a change detection when it unsubscribes.
279+
this._ngZone.runOutsideAngular(() => {
280+
// We need to wait for the zone to stabilize, in order for the reference
281+
// element to be in the proper place in the DOM. This is mostly relevant
282+
// for draggable elements inside portals since they get stamped out in
283+
// their original DOM position and then they get transferred to the portal.
284+
this._ngZone.onStable
285+
.pipe(take(1), takeUntil(this._destroyed))
286+
.subscribe(() => {
287+
this._updateRootElement();
288+
this._setupHandlesListener();
289+
290+
if (this.freeDragPosition) {
291+
this._dragRef.setFreeDragPosition(this.freeDragPosition);
292+
}
316293
});
317-
318-
if (this.freeDragPosition) {
319-
this._dragRef.setFreeDragPosition(this.freeDragPosition);
320-
}
321-
});
294+
});
322295
}
323296

324297
ngOnChanges(changes: SimpleChanges) {
@@ -346,9 +319,13 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
346319
if (index > -1) {
347320
CdkDrag._dragInstances.splice(index, 1);
348321
}
349-
this._destroyed.next();
350-
this._destroyed.complete();
351-
this._dragRef.dispose();
322+
323+
// Unnecessary in most cases, but used to avoid extra change detections with `zone-paths-rxjs`.
324+
this._ngZone.runOutsideAngular(() => {
325+
this._destroyed.next();
326+
this._destroyed.complete();
327+
this._dragRef.dispose();
328+
});
352329
}
353330

354331
/** Syncs the root element with the `DragRef`. */
@@ -536,6 +513,41 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
536513
}
537514
}
538515

516+
/** Sets up the listener that syncs the handles with the drag ref. */
517+
private _setupHandlesListener() {
518+
// Listen for any newly-added handles.
519+
this._handles.changes.pipe(
520+
startWith(this._handles),
521+
// Sync the new handles with the DragRef.
522+
tap((handles: QueryList<CdkDragHandle>) => {
523+
const childHandleElements = handles
524+
.filter(handle => handle._parentDrag === this)
525+
.map(handle => handle.element);
526+
527+
// Usually handles are only allowed to be a descendant of the drag element, but if
528+
// the consumer defined a different drag root, we should allow the drag element
529+
// itself to be a handle too.
530+
if (this._selfHandle && this.rootElementSelector) {
531+
childHandleElements.push(this.element);
532+
}
533+
534+
this._dragRef.withHandles(childHandleElements);
535+
}),
536+
// Listen if the state of any of the handles changes.
537+
switchMap((handles: QueryList<CdkDragHandle>) => {
538+
return merge(...handles.map(item => {
539+
return item._stateChanges.pipe(startWith(item));
540+
})) as Observable<CdkDragHandle>;
541+
}),
542+
takeUntil(this._destroyed)
543+
).subscribe(handleInstance => {
544+
// Enabled/disable the handle that changed in the DragRef.
545+
const dragRef = this._dragRef;
546+
const handle = handleInstance.element.nativeElement;
547+
handleInstance.disabled ? dragRef.disableHandle(handle) : dragRef.enableHandle(handle);
548+
});
549+
}
550+
539551
static ngAcceptInputType_disabled: BooleanInput;
540552
}
541553

0 commit comments

Comments
 (0)