Skip to content

Commit 5fe233a

Browse files
crisbetommalerba
authored andcommitted
fix(datepicker): memory leak in popup mode (#18102)
Fixes the datepicker leaking memory when it is opened and closed in popup mode. The leak seems to come from the fact that we're detaching, rather than disposing of, the overlay when it is closed. We did this in the past to support an animation while closing the calendar, but it seems to cause the animations engine further down the line to retain the elements memory since they aren't being removed immediately. These changes resolve the issue by triggering the exit animation, waiting for it to finish and disposing of the overlay. We weren't gaining too much by reusing the overlay from the previous approach, because the calendar was being removed and re-created every time anyway. Fixes #17896.
1 parent fba22bc commit 5fe233a

File tree

2 files changed

+65
-51
lines changed

2 files changed

+65
-51
lines changed

src/material/datepicker/datepicker.ts

Lines changed: 58 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
ViewChild,
3636
ViewContainerRef,
3737
ViewEncapsulation,
38+
ChangeDetectorRef,
3839
} from '@angular/core';
3940
import {
4041
CanColor,
@@ -92,7 +93,8 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent
9293
styleUrls: ['datepicker-content.css'],
9394
host: {
9495
'class': 'mat-datepicker-content',
95-
'[@transformPanel]': '"enter"',
96+
'[@transformPanel]': '_animationState',
97+
'(@transformPanel.done)': '_animationDone.next()',
9698
'[class.mat-datepicker-content-touch]': 'datepicker.touchUi',
9799
},
98100
animations: [
@@ -105,7 +107,7 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent
105107
inputs: ['color'],
106108
})
107109
export class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase
108-
implements AfterViewInit, CanColor {
110+
implements AfterViewInit, OnDestroy, CanColor {
109111

110112
/** Reference to the internal calendar component. */
111113
@ViewChild(MatCalendar) _calendar: MatCalendar<D>;
@@ -116,13 +118,38 @@ export class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase
116118
/** Whether the datepicker is above or below the input. */
117119
_isAbove: boolean;
118120

119-
constructor(elementRef: ElementRef) {
121+
/** Current state of the animation. */
122+
_animationState: 'enter' | 'void' = 'enter';
123+
124+
/** Emits when an animation has finished. */
125+
_animationDone = new Subject<void>();
126+
127+
constructor(
128+
elementRef: ElementRef,
129+
/**
130+
* @deprecated `_changeDetectorRef` parameter to become required.
131+
* @breaking-change 11.0.0
132+
*/
133+
private _changeDetectorRef?: ChangeDetectorRef) {
120134
super(elementRef);
121135
}
122136

123137
ngAfterViewInit() {
124138
this._calendar.focusActiveCell();
125139
}
140+
141+
ngOnDestroy() {
142+
this._animationDone.complete();
143+
}
144+
145+
_startExitAnimation() {
146+
this._animationState = 'void';
147+
148+
// @breaking-change 11.0.0 Remove null check for `_changeDetectorRef`.
149+
if (this._changeDetectorRef) {
150+
this._changeDetectorRef.markForCheck();
151+
}
152+
}
126153
}
127154

128155

@@ -250,14 +277,11 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
250277
}
251278

252279
/** A reference to the overlay when the calendar is opened as a popup. */
253-
_popupRef: OverlayRef;
280+
private _popupRef: OverlayRef | null;
254281

255282
/** A reference to the dialog when the calendar is opened as a dialog. */
256283
private _dialogRef: MatDialogRef<MatDatepickerContent<D>> | null;
257284

258-
/** A portal containing the calendar for this datepicker. */
259-
private _calendarPortal: ComponentPortal<MatDatepickerContent<D>>;
260-
261285
/** Reference to the component instantiated in popup mode. */
262286
private _popupComponentRef: ComponentRef<MatDatepickerContent<D>> | null;
263287

@@ -292,14 +316,10 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
292316
}
293317

294318
ngOnDestroy() {
319+
this._destroyPopup();
295320
this.close();
296321
this._inputSubscription.unsubscribe();
297322
this._disabledChange.complete();
298-
299-
if (this._popupRef) {
300-
this._popupRef.dispose();
301-
this._popupComponentRef = null;
302-
}
303323
}
304324

305325
/** Selects the given date */
@@ -356,16 +376,15 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
356376
if (!this._opened) {
357377
return;
358378
}
359-
if (this._popupRef && this._popupRef.hasAttached()) {
360-
this._popupRef.detach();
379+
if (this._popupComponentRef && this._popupRef) {
380+
const instance = this._popupComponentRef.instance;
381+
instance._startExitAnimation();
382+
instance._animationDone.pipe(take(1)).subscribe(() => this._destroyPopup());
361383
}
362384
if (this._dialogRef) {
363385
this._dialogRef.close();
364386
this._dialogRef = null;
365387
}
366-
if (this._calendarPortal && this._calendarPortal.isAttached) {
367-
this._calendarPortal.detach();
368-
}
369388

370389
const completeClose = () => {
371390
// The `_opened` could've been reset already if
@@ -409,30 +428,24 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
409428

410429
this._dialogRef.afterClosed().subscribe(() => this.close());
411430
this._dialogRef.componentInstance.datepicker = this;
412-
this._setColor();
431+
this._dialogRef.componentInstance.color = this.color;
413432
}
414433

415434
/** Open the calendar as a popup. */
416435
private _openAsPopup(): void {
417-
if (!this._calendarPortal) {
418-
this._calendarPortal = new ComponentPortal<MatDatepickerContent<D>>(MatDatepickerContent,
419-
this._viewContainerRef);
420-
}
421-
422-
if (!this._popupRef) {
423-
this._createPopup();
424-
}
425-
426-
if (!this._popupRef.hasAttached()) {
427-
this._popupComponentRef = this._popupRef.attach(this._calendarPortal);
428-
this._popupComponentRef.instance.datepicker = this;
429-
this._setColor();
430-
431-
// Update the position once the calendar has rendered.
432-
this._ngZone.onStable.asObservable().pipe(take(1)).subscribe(() => {
433-
this._popupRef.updatePosition();
434-
});
435-
}
436+
const portal = new ComponentPortal<MatDatepickerContent<D>>(MatDatepickerContent,
437+
this._viewContainerRef);
438+
439+
this._destroyPopup();
440+
this._createPopup();
441+
const ref = this._popupComponentRef = this._popupRef!.attach(portal);
442+
ref.instance.datepicker = this;
443+
ref.instance.color = this.color;
444+
445+
// Update the position once the calendar has rendered.
446+
this._ngZone.onStable.asObservable().pipe(take(1)).subscribe(() => {
447+
this._popupRef!.updatePosition();
448+
});
436449
}
437450

438451
/** Create the popup. */
@@ -466,6 +479,14 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
466479
});
467480
}
468481

482+
/** Destroys the current popup overlay. */
483+
private _destroyPopup() {
484+
if (this._popupRef) {
485+
this._popupRef.dispose();
486+
this._popupRef = this._popupComponentRef = null;
487+
}
488+
}
489+
469490
/** Create the popup PositionStrategy. */
470491
private _createPopupPositionStrategy(): PositionStrategy {
471492
return this._overlay.position()
@@ -510,17 +531,6 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
510531
return (this._dateAdapter.isDateInstance(obj) && this._dateAdapter.isValid(obj)) ? obj : null;
511532
}
512533

513-
/** Passes the current theme color along to the calendar overlay. */
514-
private _setColor(): void {
515-
const color = this.color;
516-
if (this._popupComponentRef) {
517-
this._popupComponentRef.instance.color = color;
518-
}
519-
if (this._dialogRef) {
520-
this._dialogRef.componentInstance.color = color;
521-
}
522-
}
523-
524534
static ngAcceptInputType_disabled: BooleanInput;
525535
static ngAcceptInputType_touchUi: BooleanInput;
526536
}

tools/public_api_guard/material/datepicker.d.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ export declare class MatDatepicker<D> implements OnDestroy, CanColor {
108108
readonly _disabledChange: Subject<boolean>;
109109
readonly _maxDate: D | null;
110110
readonly _minDate: D | null;
111-
_popupRef: OverlayRef;
112111
_selected: D | null;
113112
readonly _selectedChanged: Subject<D>;
114113
calendarHeaderComponent: ComponentType<any>;
@@ -144,12 +143,17 @@ export declare const matDatepickerAnimations: {
144143
readonly fadeInCalendar: AnimationTriggerMetadata;
145144
};
146145

147-
export declare class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase implements AfterViewInit, CanColor {
146+
export declare class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase implements AfterViewInit, OnDestroy, CanColor {
147+
_animationDone: Subject<void>;
148+
_animationState: 'enter' | 'void';
148149
_calendar: MatCalendar<D>;
149150
_isAbove: boolean;
150151
datepicker: MatDatepicker<D>;
151-
constructor(elementRef: ElementRef);
152+
constructor(elementRef: ElementRef,
153+
_changeDetectorRef?: ChangeDetectorRef | undefined);
154+
_startExitAnimation(): void;
152155
ngAfterViewInit(): void;
156+
ngOnDestroy(): void;
153157
static ɵcmp: i0.ɵɵComponentDefWithMeta<MatDatepickerContent<any>, "mat-datepicker-content", ["matDatepickerContent"], { "color": "color"; }, {}, never>;
154158
static ɵfac: i0.ɵɵFactoryDef<MatDatepickerContent<any>>;
155159
}

0 commit comments

Comments
 (0)