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.

Sign up for GitHub

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(datepicker): memory leak in popup mode #18102

Merged
merged 1 commit into from
Jan 10, 2020
Merged

fix(datepicker): memory leak in popup mode #18102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
106 changes: 58 additions & 48 deletions src/material/datepicker/datepicker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
ViewChild,
ViewContainerRef,
ViewEncapsulation,
ChangeDetectorRef,
} from '@angular/core';
import {
CanColor,
Expand Down Expand Up @@ -92,7 +93,8 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent
styleUrls: ['datepicker-content.css'],
host: {
'class': 'mat-datepicker-content',
'[@transformPanel]': '"enter"',
'[@transformPanel]': '_animationState',
'(@transformPanel.done)': '_animationDone.next()',
'[class.mat-datepicker-content-touch]': 'datepicker.touchUi',
},
animations: [
Expand All @@ -105,7 +107,7 @@ const _MatDatepickerContentMixinBase: CanColorCtor & typeof MatDatepickerContent
inputs: ['color'],
})
export class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase
implements AfterViewInit, CanColor {
implements AfterViewInit, OnDestroy, CanColor {

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

constructor(elementRef: ElementRef) {
/** Current state of the animation. */
_animationState: 'enter' | 'void' = 'enter';

/** Emits when an animation has finished. */
_animationDone = new Subject<void>();

constructor(
elementRef: ElementRef,
/**
* @deprecated `_changeDetectorRef` parameter to become required.
* @breaking-change 11.0.0
*/
private _changeDetectorRef?: ChangeDetectorRef) {
super(elementRef);
}

ngAfterViewInit() {
this._calendar.focusActiveCell();
}

ngOnDestroy() {
this._animationDone.complete();
}

_startExitAnimation() {
this._animationState = 'void';

// @breaking-change 11.0.0 Remove null check for `_changeDetectorRef`.
if (this._changeDetectorRef) {
this._changeDetectorRef.markForCheck();
}
}
}


Expand Down Expand Up @@ -250,14 +277,11 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
}

/** A reference to the overlay when the calendar is opened as a popup. */
_popupRef: OverlayRef;
private _popupRef: OverlayRef | null;

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

/** A portal containing the calendar for this datepicker. */
private _calendarPortal: ComponentPortal<MatDatepickerContent<D>>;

/** Reference to the component instantiated in popup mode. */
private _popupComponentRef: ComponentRef<MatDatepickerContent<D>> | null;

Expand Down Expand Up @@ -292,14 +316,10 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
}

ngOnDestroy() {
this._destroyPopup();
this.close();
this._inputSubscription.unsubscribe();
this._disabledChange.complete();

if (this._popupRef) {
this._popupRef.dispose();
this._popupComponentRef = null;
}
}

/** Selects the given date */
Expand Down Expand Up @@ -356,16 +376,15 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
if (!this._opened) {
return;
}
if (this._popupRef && this._popupRef.hasAttached()) {
this._popupRef.detach();
if (this._popupComponentRef && this._popupRef) {
const instance = this._popupComponentRef.instance;
instance._startExitAnimation();
instance._animationDone.pipe(take(1)).subscribe(() => this._destroyPopup());
}
if (this._dialogRef) {
this._dialogRef.close();
this._dialogRef = null;
}
if (this._calendarPortal && this._calendarPortal.isAttached) {
this._calendarPortal.detach();
}

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

this._dialogRef.afterClosed().subscribe(() => this.close());
this._dialogRef.componentInstance.datepicker = this;
this._setColor();
this._dialogRef.componentInstance.color = this.color;
}

/** Open the calendar as a popup. */
private _openAsPopup(): void {
if (!this._calendarPortal) {
this._calendarPortal = new ComponentPortal<MatDatepickerContent<D>>(MatDatepickerContent,
this._viewContainerRef);
}

if (!this._popupRef) {
this._createPopup();
}

if (!this._popupRef.hasAttached()) {
this._popupComponentRef = this._popupRef.attach(this._calendarPortal);
this._popupComponentRef.instance.datepicker = this;
this._setColor();

// Update the position once the calendar has rendered.
this._ngZone.onStable.asObservable().pipe(take(1)).subscribe(() => {
this._popupRef.updatePosition();
});
}
const portal = new ComponentPortal<MatDatepickerContent<D>>(MatDatepickerContent,
this._viewContainerRef);

this._destroyPopup();
this._createPopup();
const ref = this._popupComponentRef = this._popupRef!.attach(portal);
ref.instance.datepicker = this;
ref.instance.color = this.color;

// Update the position once the calendar has rendered.
this._ngZone.onStable.asObservable().pipe(take(1)).subscribe(() => {
this._popupRef!.updatePosition();
});
}

/** Create the popup. */
Expand Down Expand Up @@ -466,6 +479,14 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
});
}

/** Destroys the current popup overlay. */
private _destroyPopup() {
if (this._popupRef) {
this._popupRef.dispose();
this._popupRef = this._popupComponentRef = null;
}
}

/** Create the popup PositionStrategy. */
private _createPopupPositionStrategy(): PositionStrategy {
return this._overlay.position()
Expand Down Expand Up @@ -510,17 +531,6 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
return (this._dateAdapter.isDateInstance(obj) && this._dateAdapter.isValid(obj)) ? obj : null;
}

/** Passes the current theme color along to the calendar overlay. */
private _setColor(): void {
const color = this.color;
if (this._popupComponentRef) {
this._popupComponentRef.instance.color = color;
}
if (this._dialogRef) {
this._dialogRef.componentInstance.color = color;
}
}

static ngAcceptInputType_disabled: BooleanInput;
static ngAcceptInputType_touchUi: BooleanInput;
}
10 changes: 7 additions & 3 deletions tools/public_api_guard/material/datepicker.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ export declare class MatDatepicker<D> implements OnDestroy, CanColor {
readonly _disabledChange: Subject<boolean>;
readonly _maxDate: D | null;
readonly _minDate: D | null;
_popupRef: OverlayRef;
_selected: D | null;
readonly _selectedChanged: Subject<D>;
calendarHeaderComponent: ComponentType<any>;
Expand Down Expand Up @@ -144,12 +143,17 @@ export declare const matDatepickerAnimations: {
readonly fadeInCalendar: AnimationTriggerMetadata;
};

export declare class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase implements AfterViewInit, CanColor {
export declare class MatDatepickerContent<D> extends _MatDatepickerContentMixinBase implements AfterViewInit, OnDestroy, CanColor {
_animationDone: Subject<void>;
_animationState: 'enter' | 'void';
_calendar: MatCalendar<D>;
_isAbove: boolean;
datepicker: MatDatepicker<D>;
constructor(elementRef: ElementRef);
constructor(elementRef: ElementRef,
_changeDetectorRef?: ChangeDetectorRef | undefined);
_startExitAnimation(): void;
ngAfterViewInit(): void;
ngOnDestroy(): void;
static ɵcmp: i0.ɵɵComponentDefWithMeta<MatDatepickerContent<any>, "mat-datepicker-content", ["matDatepickerContent"], { "color": "color"; }, {}, never>;
static ɵfac: i0.ɵɵFactoryDef<MatDatepickerContent<any>>;
}
Expand Down