fix(icon): not updating svg icon assigned through setter (#20509)

Moves some logic around so the icon can respond to changes in its state through plain property assignments, rather than going through Angular change detection cycle. This makes the component easier to extend.

Fixes #20470.

(cherry picked from commit 4263cac)

Author: crisbeto

src/material/icon/icon.spec.ts

@@ -5,11 +5,12 @@ import {
   HttpTestingController,
   TestRequest,
 } from '@angular/common/http/testing';
-import {Component, ErrorHandler} from '@angular/core';
+import {Component, ErrorHandler, ViewChild} from '@angular/core';
 import {MatIconModule, MAT_ICON_LOCATION} from './index';
 import {MatIconRegistry, getMatIconNoHttpProviderError} from './icon-registry';
 import {FAKE_SVGS} from './fake-svgs';
 import {wrappedErrorMessage} from '@angular/cdk/testing/private';
+import {MatIcon} from './icon';
 
 
 /** Returns the CSS classes assigned to an element as a sorted array. */
@@ -64,6 +65,7 @@ describe('MatIcon', () => {
         InlineIcon,
         SvgIconWithUserContent,
         IconWithLigatureAndSvgBinding,
+        BlankIcon,
       ],
       providers: [
         {
@@ -999,6 +1001,22 @@ describe('MatIcon', () => {
 
   });
 
+  it('should handle assigning an icon through the setter', fakeAsync(() => {
+    iconRegistry.addSvgIconLiteral('fido', trustHtml(FAKE_SVGS.dog));
+
+    const fixture = TestBed.createComponent(BlankIcon);
+    fixture.detectChanges();
+    let svgElement: SVGElement;
+    const testComponent = fixture.componentInstance;
+    const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
+
+    testComponent.icon.svgIcon = 'fido';
+    fixture.detectChanges();
+    svgElement = verifyAndGetSingleSvgChild(iconElement);
+    verifyPathChildElement(svgElement, 'woof');
+    tick();
+  }));
+
   /** Marks an SVG icon url as explicitly trusted. */
   function trustUrl(iconUrl: string): SafeResourceUrl {
     return sanitizer.bypassSecurityTrustResourceUrl(iconUrl);
@@ -1089,3 +1107,8 @@ class SvgIconWithUserContent {
 class IconWithLigatureAndSvgBinding {
   iconName: string | undefined;
 }
+
+@Component({template: `<mat-icon></mat-icon>`})
+class BlankIcon {
+  @ViewChild(MatIcon) icon: MatIcon;
+}

src/material/icon/icon.ts

@@ -19,10 +19,8 @@ import {
   Inject,
   InjectionToken,
   Input,
-  OnChanges,
   OnDestroy,
   OnInit,
-  SimpleChanges,
   ViewEncapsulation,
 } from '@angular/core';
 import {CanColor, CanColorCtor, mixinColor} from '@angular/material/core';
@@ -137,8 +135,8 @@ const funcIriPattern = /^url\(['"]?#(.*?)['"]?\)$/;
   encapsulation: ViewEncapsulation.None,
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, AfterViewChecked,
-  CanColor, OnDestroy {
+export class MatIcon extends _MatIconMixinBase implements OnInit, AfterViewChecked, CanColor,
+  OnDestroy {
 
   /**
    * Whether the icon should be inlined, automatically sizing the icon to match the font size of
@@ -154,21 +152,43 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
   private _inline: boolean = false;
 
   /** Name of the icon in the SVG icon set. */
-  @Input() svgIcon: string;
+  @Input()
+  get svgIcon(): string { return this._svgIcon; }
+  set svgIcon(value: string) {
+    if (value !== this._svgIcon) {
+      if (value) {
+        this._updateSvgIcon(value);
+      } else if (this._svgIcon) {
+        this._clearSvgElement();
+      }
+      this._svgIcon = value;
+    }
+  }
+  private _svgIcon: string;
 
   /** Font set that the icon is a part of. */
   @Input()
   get fontSet(): string { return this._fontSet; }
   set fontSet(value: string) {
-    this._fontSet = this._cleanupFontValue(value);
+    const newValue = this._cleanupFontValue(value);
+
+    if (newValue !== this._fontSet) {
+      this._fontSet = newValue;
+      this._updateFontIconClasses();
+    }
   }
   private _fontSet: string;
 
   /** Name of an icon within a font set. */
   @Input()
   get fontIcon(): string { return this._fontIcon; }
   set fontIcon(value: string) {
-    this._fontIcon = this._cleanupFontValue(value);
+    const newValue = this._cleanupFontValue(value);
+
+    if (newValue !== this._fontIcon) {
+      this._fontIcon = newValue;
+      this._updateFontIconClasses();
+    }
   }
   private _fontIcon: string;
 
@@ -226,49 +246,10 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
     }
   }
 
-  ngOnChanges(changes: SimpleChanges) {
-    // Only update the inline SVG icon if the inputs changed, to avoid unnecessary DOM operations.
-    const svgIconChanges = changes['svgIcon'];
-
-    this._svgNamespace = null;
-    this._svgName = null;
-
-    if (svgIconChanges) {
-      this._currentIconFetch.unsubscribe();
-
-      if (this.svgIcon) {
-        const [namespace, iconName] = this._splitIconName(this.svgIcon);
-
-        if (namespace) {
-          this._svgNamespace = namespace;
-        }
-
-        if (iconName) {
-          this._svgName = iconName;
-        }
-
-        this._currentIconFetch = this._iconRegistry.getNamedSvgIcon(iconName, namespace)
-            .pipe(take(1))
-            .subscribe(svg => this._setSvgElement(svg), (err: Error) => {
-              const errorMessage = `Error retrieving icon ${namespace}:${iconName}! ${err.message}`;
-              this._errorHandler.handleError(new Error(errorMessage));
-            });
-      } else if (svgIconChanges.previousValue) {
-        this._clearSvgElement();
-      }
-    }
-
-    if (this._usingFontIcon()) {
-      this._updateFontIconClasses();
-    }
-  }
-
   ngOnInit() {
     // Update font classes because ngOnChanges won't be called if none of the inputs are present,
     // e.g. <mat-icon>arrow</mat-icon> In this case we need to add a CSS class for the default font.
-    if (this._usingFontIcon()) {
-      this._updateFontIconClasses();
-    }
+    this._updateFontIconClasses();
   }
 
   ngAfterViewChecked() {
@@ -430,5 +411,31 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
     }
   }
 
+  /** Sets a new SVG icon with a particular name. */
+  private _updateSvgIcon(rawName: string|undefined) {
+    this._svgNamespace = null;
+    this._svgName = null;
+    this._currentIconFetch.unsubscribe();
+
+    if (rawName) {
+      const [namespace, iconName] = this._splitIconName(rawName);
+
+      if (namespace) {
+        this._svgNamespace = namespace;
+      }
+
+      if (iconName) {
+        this._svgName = iconName;
+      }
+
+      this._currentIconFetch = this._iconRegistry.getNamedSvgIcon(iconName, namespace)
+          .pipe(take(1))
+          .subscribe(svg => this._setSvgElement(svg), (err: Error) => {
+            const errorMessage = `Error retrieving icon ${namespace}:${iconName}! ${err.message}`;
+            this._errorHandler.handleError(new Error(errorMessage));
+          });
+    }
+  }
+
   static ngAcceptInputType_inline: BooleanInput;
 }

tools/public_api_guard/material/icon.d.ts

@@ -23,7 +23,7 @@ export declare const MAT_ICON_LOCATION: InjectionToken<MatIconLocation>;
 
 export declare function MAT_ICON_LOCATION_FACTORY(): MatIconLocation;
 
-export declare class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, AfterViewChecked, CanColor, OnDestroy {
+export declare class MatIcon extends _MatIconMixinBase implements OnInit, AfterViewChecked, CanColor, OnDestroy {
     _svgName: string | null;
     _svgNamespace: string | null;
     get fontIcon(): string;
@@ -32,11 +32,11 @@ export declare class MatIcon extends _MatIconMixinBase implements OnChanges, OnI
     set fontSet(value: string);
     get inline(): boolean;
     set inline(inline: boolean);
-    svgIcon: string;
+    get svgIcon(): string;
+    set svgIcon(value: string);
     constructor(elementRef: ElementRef<HTMLElement>, _iconRegistry: MatIconRegistry, ariaHidden: string, _location: MatIconLocation, _errorHandler: ErrorHandler);
     _usingFontIcon(): boolean;
     ngAfterViewChecked(): void;
-    ngOnChanges(changes: SimpleChanges): void;
     ngOnDestroy(): void;
     ngOnInit(): void;
     static ngAcceptInputType_inline: BooleanInput;