Skip to content

Commit 0baca18

Browse files
authored
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.
1 parent 45f3bf9 commit 0baca18

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`. */
@@ -535,5 +512,40 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
535512
}
536513
}
537514

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

0 commit comments

Comments
 (0)