Skip to content

Commit 35173f1

Browse files
committed
fix(cdk/overlay): avoid over-subscribing to afterNextRender
Currently the `OverlayRef` subscribes to `afterNextRender` on init and unsubscribes on dispose. This isn't necessary, because we only use `afterNextRender` during the `dispose` sequence. These changes switch to only subscribing when we need to.
1 parent f4cb9f1 commit 35173f1

File tree

1 file changed

+32
-41
lines changed

1 file changed

+32
-41
lines changed

src/cdk/overlay/overlay-ref.ts

Lines changed: 32 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,10 @@ import {
1616
afterNextRender,
1717
afterRender,
1818
untracked,
19-
AfterRenderRef,
2019
} from '@angular/core';
2120
import {Location} from '@angular/common';
2221
import {Observable, Subject, merge, SubscriptionLike, Subscription} from 'rxjs';
23-
import {takeUntil} from 'rxjs/operators';
22+
import {take} from 'rxjs/operators';
2423
import {OverlayKeyboardDispatcher} from './dispatchers/overlay-keyboard-dispatcher';
2524
import {OverlayOutsideClickDispatcher} from './dispatchers/overlay-outside-click-dispatcher';
2625
import {OverlayConfig} from './overlay-config';
@@ -39,7 +38,7 @@ export type ImmutableObject<T> = {
3938
*/
4039
export class OverlayRef implements PortalOutlet {
4140
private _backdropElement: HTMLElement | null = null;
42-
private _backdropTimeout: number | undefined;
41+
private _backdropTimeout: ReturnType<typeof setTimeout> | undefined;
4342
private readonly _backdropClick = new Subject<MouseEvent>();
4443
private readonly _attachments = new Subject<void>();
4544
private readonly _detachments = new Subject<void>();
@@ -63,10 +62,6 @@ export class OverlayRef implements PortalOutlet {
6362
/** Stream of mouse outside events dispatched to this overlay. */
6463
readonly _outsidePointerEvents = new Subject<MouseEvent>();
6564

66-
private _renders = new Subject<void>();
67-
68-
private _afterRenderRef: AfterRenderRef;
69-
7065
constructor(
7166
private _portalOutlet: PortalOutlet,
7267
private _host: HTMLElement,
@@ -86,18 +81,6 @@ export class OverlayRef implements PortalOutlet {
8681
}
8782

8883
this._positionStrategy = _config.positionStrategy;
89-
90-
// Users could open the overlay from an `effect`, in which case we need to
91-
// run the `afterRender` as `untracked`. We don't recommend that users do
92-
// this, but we also don't want to break users who are doing it.
93-
this._afterRenderRef = untracked(() =>
94-
afterRender(
95-
() => {
96-
this._renders.next();
97-
},
98-
{injector: this._injector},
99-
),
100-
);
10184
}
10285

10386
/** The overlay's HTML element */
@@ -275,8 +258,6 @@ export class OverlayRef implements PortalOutlet {
275258
}
276259

277260
this._detachments.complete();
278-
this._afterRenderRef.destroy();
279-
this._renders.complete();
280261
}
281262

282263
/** Whether the overlay has attached content. */
@@ -516,28 +497,38 @@ export class OverlayRef implements PortalOutlet {
516497
// Normally we wouldn't have to explicitly run this outside the `NgZone`, however
517498
// if the consumer is using `zone-patch-rxjs`, the `Subscription.unsubscribe` call will
518499
// be patched to run inside the zone, which will throw us into an infinite loop.
500+
// We can't remove the host here immediately, because the overlay pane's content
501+
// might still be animating. This stream helps us avoid interrupting the animation
502+
// by waiting for the pane to become empty.
519503
this._ngZone.runOutsideAngular(() => {
520-
// We can't remove the host here immediately, because the overlay pane's content
521-
// might still be animating. This stream helps us avoid interrupting the animation
522-
// by waiting for the pane to become empty.
523-
const subscription = this._renders
524-
.pipe(takeUntil(merge(this._attachments, this._detachments)))
525-
.subscribe(() => {
526-
// Needs a couple of checks for the pane and host, because
527-
// they may have been removed by the time the zone stabilizes.
528-
if (!this._pane || !this._host || this._pane.children.length === 0) {
529-
if (this._pane && this._config.panelClass) {
530-
this._toggleClasses(this._pane, this._config.panelClass, false);
504+
// Users could open the overlay from an `effect`, in which case we need to
505+
// run the `afterRender` as `untracked`. We don't recommend that users do
506+
// this, but we also don't want to break users who are doing it.
507+
const ref = untracked(() =>
508+
afterRender(
509+
() => {
510+
// Needs a couple of checks for the pane and host, because
511+
// they may have been removed by the time the zone stabilizes.
512+
if (!this._pane || !this._host || this._pane.children.length === 0) {
513+
if (this._pane && this._config.panelClass) {
514+
this._toggleClasses(this._pane, this._config.panelClass, false);
515+
}
516+
517+
if (this._host && this._host.parentElement) {
518+
this._previousHostParent = this._host.parentElement;
519+
this._host.remove();
520+
}
521+
522+
ref.destroy();
531523
}
532-
533-
if (this._host && this._host.parentElement) {
534-
this._previousHostParent = this._host.parentElement;
535-
this._host.remove();
536-
}
537-
538-
subscription.unsubscribe();
539-
}
540-
});
524+
},
525+
{injector: this._injector},
526+
),
527+
);
528+
529+
merge(this._attachments, this._detachments)
530+
.pipe(take(1))
531+
.subscribe(() => ref.destroy());
541532
});
542533
}
543534

0 commit comments

Comments
 (0)