fix(tooltip): clear pending timers on destroy

Currently we fire off some timers when a tooltip is shown or hidden, however if it gets destroyed before the timer has elapsed, we still end up executing some logic (change detection in this case). These changes clear the timers so that we don't end up triggering errors by accident.

Author: crisbeto

src/material/tooltip/tooltip.spec.ts

@@ -111,17 +111,17 @@ describe('MatTooltip', () => {
 
       fixture.detectChanges();
 
-      // wait till animation has finished
+      // Wait until the animation has finished.
       tick(500);
 
-      // Make sure tooltip is shown to the user and animation has finished
+      // Make sure tooltip is shown to the user and animation has finished.
       const tooltipElement = overlayContainerElement.querySelector('.mat-tooltip') as HTMLElement;
       expect(tooltipElement instanceof HTMLElement).toBe(true);
       expect(tooltipElement.style.transform).toBe('scale(1)');
 
       expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
 
-      // After hide called, a timeout delay is created that will to hide the tooltip.
+      // After hide is called, a timeout delay is created that will to hide the tooltip.
       const tooltipDelay = 1000;
       tooltipDirective.hide(tooltipDelay);
       expect(tooltipDirective._isTooltipVisible()).toBe(true);
@@ -719,6 +719,33 @@ describe('MatTooltip', () => {
       expect(overlayRef.detach).not.toHaveBeenCalled();
     }));
 
+    it('should clear the show timeout on destroy', fakeAsync(() => {
+      assertTooltipInstance(tooltipDirective, false);
+
+      tooltipDirective.show(1000);
+      fixture.detectChanges();
+
+      // Note that we aren't asserting anything, but `fakeAsync` will
+      // throw if we have any timers by the end of the test.
+      fixture.destroy();
+    }));
+
+    it('should clear the hide timeout on destroy', fakeAsync(() => {
+      assertTooltipInstance(tooltipDirective, false);
+
+      tooltipDirective.show();
+      tick(0);
+      fixture.detectChanges();
+      tick(500);
+
+      tooltipDirective.hide(1000);
+      fixture.detectChanges();
+
+      // Note that we aren't asserting anything, but `fakeAsync` will
+      // throw if we have any timers by the end of the test.
+      fixture.destroy();
+    }));
+
   });
 
   describe('fallback positions', () => {

src/material/tooltip/tooltip.ts

@@ -550,10 +550,10 @@ export class TooltipComponent implements OnDestroy {
   tooltipClass: string|string[]|Set<string>|{[key: string]: any};
 
   /** The timeout ID of any current timer set to show the tooltip */
-  _showTimeoutId: number | null;
+  _showTimeoutId: number | undefined;
 
   /** The timeout ID of any current timer set to hide the tooltip */
-  _hideTimeoutId: number | null;
+  _hideTimeoutId: number | undefined;
 
   /** Property watched by the animation framework to show or hide the tooltip */
   _visibility: TooltipVisibility = 'initial';
@@ -577,16 +577,13 @@ export class TooltipComponent implements OnDestroy {
    */
   show(delay: number): void {
     // Cancel the delayed hide if it is scheduled
-    if (this._hideTimeoutId) {
-      clearTimeout(this._hideTimeoutId);
-      this._hideTimeoutId = null;
-    }
+    clearTimeout(this._hideTimeoutId);
 
     // Body interactions should cancel the tooltip if there is a delay in showing.
     this._closeOnInteraction = true;
     this._showTimeoutId = setTimeout(() => {
       this._visibility = 'visible';
-      this._showTimeoutId = null;
+      this._showTimeoutId = undefined;
 
       // Mark for check so if any parent component has set the
       // ChangeDetectionStrategy to OnPush it will be checked anyways
@@ -600,14 +597,11 @@ export class TooltipComponent implements OnDestroy {
    */
   hide(delay: number): void {
     // Cancel the delayed show if it is scheduled
-    if (this._showTimeoutId) {
-      clearTimeout(this._showTimeoutId);
-      this._showTimeoutId = null;
-    }
+    clearTimeout(this._showTimeoutId);
 
     this._hideTimeoutId = setTimeout(() => {
       this._visibility = 'hidden';
-      this._hideTimeoutId = null;
+      this._hideTimeoutId = undefined;
 
       // Mark for check so if any parent component has set the
       // ChangeDetectionStrategy to OnPush it will be checked anyways
@@ -626,6 +620,8 @@ export class TooltipComponent implements OnDestroy {
   }
 
   ngOnDestroy() {
+    clearTimeout(this._showTimeoutId);
+    clearTimeout(this._hideTimeoutId);
     this._onHide.complete();
   }
 

tools/public_api_guard/material/tooltip.d.ts

@@ -63,9 +63,9 @@ export declare const SCROLL_THROTTLE_MS = 20;
 export declare const TOOLTIP_PANEL_CLASS = "mat-tooltip-panel";
 
 export declare class TooltipComponent implements OnDestroy {
-    _hideTimeoutId: number | null;
+    _hideTimeoutId: number | undefined;
     _isHandset: Observable<BreakpointState>;
-    _showTimeoutId: number | null;
+    _showTimeoutId: number | undefined;
     _visibility: TooltipVisibility;
     message: string;
     tooltipClass: string | string[] | Set<string> | {