Revert "fix(icon): use ErrorHandler to log MatIcon errors (angular#16967

)" (angular#16987) This reverts commit dccddd9. Temporarily reverting because this causes a failure in a Google Cloud test that didn't show up on the presubmit. Will undo this revert once we can resolve that failure.
jelbourn · Sep 5, 2019 · 680ed00 · 680ed00
1 parent 7b12a81
commit 680ed00
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 53 deletions.
diff --git a/src/material/icon/icon.spec.ts b/src/material/icon/icon.spec.ts
@@ -278,35 +278,10 @@ describe('MatIcon', () => {
       http.expectOne('farm-set-1.svg').error(new ErrorEvent('Network error'));
       fixture.detectChanges();
 
-      // Called twice once for the HTTP request failing and once for the icon
-      // then not being able to be found.
-      expect(errorHandler.handleError).toHaveBeenCalledTimes(2);
+      expect(errorHandler.handleError).toHaveBeenCalledTimes(1);
       expect(errorHandler.handleError.calls.argsFor(0)[0].message).toEqual(
         'Loading icon set URL: farm-set-1.svg failed: Http failure response ' +
         'for farm-set-1.svg: 0 ');
-      expect(errorHandler.handleError.calls.argsFor(1)[0].message)
-          .toEqual(
-              `Error retrieving icon ${testComponent.iconName}! ` +
-              'Unable to find icon with the name "pig"');
-    });
-
-    it('should delegate an error getting an SVG icon to the ErrorHandler', () => {
-      iconRegistry.addSvgIconSetInNamespace('farm', trustUrl('farm-set-1.svg'));
-
-      const fixture = TestBed.createComponent(IconFromSvgName);
-      const testComponent = fixture.componentInstance;
-
-      testComponent.iconName = 'farm:DNE';
-      fixture.detectChanges();
-      http.expectOne('farm-set-1.svg').flush(FAKE_SVGS.farmSet1);
-      fixture.detectChanges();
-
-      // The HTTP request succeeded but the icon was not found so we logged.
-      expect(errorHandler.handleError).toHaveBeenCalledTimes(1);
-      expect(errorHandler.handleError.calls.argsFor(0)[0].message)
-          .toEqual(
-              `Error retrieving icon ${testComponent.iconName}! ` +
-              'Unable to find icon with the name "DNE"');
     });
 
     it('should extract icon from SVG icon set', () => {

diff --git a/src/material/icon/icon.ts b/src/material/icon/icon.ts
@@ -6,29 +6,27 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {coerceBooleanProperty} from '@angular/cdk/coercion';
-import {DOCUMENT} from '@angular/common';
+import {take} from 'rxjs/operators';
 import {
-  AfterViewChecked,
   Attribute,
   ChangeDetectionStrategy,
   Component,
   ElementRef,
-  ErrorHandler,
-  inject,
-  Inject,
-  InjectionToken,
   Input,
   OnChanges,
-  OnDestroy,
   OnInit,
-  Optional,
   SimpleChanges,
   ViewEncapsulation,
+  Optional,
+  InjectionToken,
+  inject,
+  Inject,
+  OnDestroy,
+  AfterViewChecked,
 } from '@angular/core';
+import {DOCUMENT} from '@angular/common';
 import {CanColor, CanColorCtor, mixinColor} from '@angular/material/core';
-import {take} from 'rxjs/operators';
-
+import {coerceBooleanProperty} from '@angular/cdk/coercion';
 import {MatIconRegistry} from './icon-registry';
 
 
@@ -180,15 +178,14 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
   private _elementsWithExternalReferences?: Map<Element, {name: string, value: string}[]>;
 
   constructor(
-      elementRef: ElementRef<HTMLElement>, private _iconRegistry: MatIconRegistry,
+      elementRef: ElementRef<HTMLElement>,
+      private _iconRegistry: MatIconRegistry,
       @Attribute('aria-hidden') ariaHidden: string,
       /**
        * @deprecated `location` parameter to be made required.
        * @breaking-change 8.0.0
        */
-      @Optional() @Inject(MAT_ICON_LOCATION) private _location?: MatIconLocation,
-      // @breaking-change 9.0.0 _errorHandler parameter to be made required
-      @Optional() private readonly _errorHandler?: ErrorHandler) {
+      @Optional() @Inject(MAT_ICON_LOCATION) private _location?: MatIconLocation) {
     super(elementRef);
 
     // If the user has not explicitly set aria-hidden, mark the icon as hidden, as this is
@@ -231,17 +228,10 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
       if (this.svgIcon) {
         const [namespace, iconName] = this._splitIconName(this.svgIcon);
 
-        this._iconRegistry.getNamedSvgIcon(iconName, namespace)
-            .pipe(take(1))
-            .subscribe(svg => this._setSvgElement(svg), (err: Error) => {
-              const errorMessage = `Error retrieving icon ${namespace}:${iconName}! ${err.message}`;
-              // @breaking-change 9.0.0 _errorHandler parameter to be made required.
-              if (this._errorHandler) {
-                this._errorHandler.handleError(new Error(errorMessage));
-              } else {
-                console.error(errorMessage);
-              }
-            });
+        this._iconRegistry.getNamedSvgIcon(iconName, namespace).pipe(take(1)).subscribe(
+          svg => this._setSvgElement(svg),
+          (err: Error) => console.log(`Error retrieving icon: ${err.message}`)
+        );
       } else if (svgIconChanges.previousValue) {
         this._clearSvgElement();
       }

diff --git a/tools/public_api_guard/material/icon.d.ts b/tools/public_api_guard/material/icon.d.ts
@@ -28,7 +28,7 @@ export declare class MatIcon extends _MatIconMixinBase implements OnChanges, OnI
     inline: boolean;
     svgIcon: string;
     constructor(elementRef: ElementRef<HTMLElement>, _iconRegistry: MatIconRegistry, ariaHidden: string,
-    _location?: MatIconLocation | undefined, _errorHandler?: ErrorHandler | undefined);
+    _location?: MatIconLocation | undefined);
     ngAfterViewChecked(): void;
     ngOnChanges(changes: SimpleChanges): void;
     ngOnDestroy(): void;