Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cdk/overlay): avoid leaking memory through afterNextRender #29709

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/cdk/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
import {Direction, Directionality} from '@angular/cdk/bidi';
import {ComponentPortal, Portal, PortalOutlet, TemplatePortal} from '@angular/cdk/portal';
import {
AfterRenderRef,
ComponentRef,
EmbeddedViewRef,
EnvironmentInjector,
NgZone,
afterNextRender,
afterRender,
untracked,
AfterRenderRef,
} from '@angular/core';
import {Location} from '@angular/common';
import {Observable, Subject, merge, SubscriptionLike, Subscription} from 'rxjs';
Expand All @@ -39,7 +39,7 @@ export type ImmutableObject<T> = {
*/
export class OverlayRef implements PortalOutlet {
private _backdropElement: HTMLElement | null = null;
private _backdropTimeout: number | undefined;
private _backdropTimeout: ReturnType<typeof setTimeout> | undefined;
private readonly _backdropClick = new Subject<MouseEvent>();
private readonly _attachments = new Subject<void>();
private readonly _detachments = new Subject<void>();
Expand Down Expand Up @@ -67,6 +67,9 @@ export class OverlayRef implements PortalOutlet {

private _afterRenderRef: AfterRenderRef;

/** Reference to the currently-running `afterNextRender` call. */
private _afterNextRenderRef: AfterRenderRef | undefined;

constructor(
private _portalOutlet: PortalOutlet,
private _host: HTMLElement,
Expand Down Expand Up @@ -151,9 +154,14 @@ export class OverlayRef implements PortalOutlet {
this._scrollStrategy.enable();
}

// We need to clean this up ourselves, because we're passing in an
// `EnvironmentInjector` below which won't ever be destroyed.
// Otherwise it causes some callbacks to be retained (see #29696).
this._afterNextRenderRef?.destroy();

// Update the position once the overlay is fully rendered before attempting to position it,
// as the position may depend on the size of the rendered content.
afterNextRender(
this._afterNextRenderRef = afterNextRender(
() => {
// The overlay could've been detached before the callback executed.
if (this.hasAttached()) {
Expand Down Expand Up @@ -267,6 +275,7 @@ export class OverlayRef implements PortalOutlet {
this._outsidePointerEvents.complete();
this._outsideClickDispatcher.remove(this);
this._host?.remove();
this._afterNextRenderRef?.destroy();

this._previousHostParent = this._pane = this._host = null!;

Expand Down
Loading