Skip to content

Commit 680ed00

Browse files
authored
Revert "fix(icon): use ErrorHandler to log MatIcon errors (#16967)" (#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.
1 parent 7b12a81 commit 680ed00

File tree

3 files changed

+18
-53
lines changed

3 files changed

+18
-53
lines changed

src/material/icon/icon.spec.ts

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -278,35 +278,10 @@ describe('MatIcon', () => {
278278
http.expectOne('farm-set-1.svg').error(new ErrorEvent('Network error'));
279279
fixture.detectChanges();
280280

281-
// Called twice once for the HTTP request failing and once for the icon
282-
// then not being able to be found.
283-
expect(errorHandler.handleError).toHaveBeenCalledTimes(2);
281+
expect(errorHandler.handleError).toHaveBeenCalledTimes(1);
284282
expect(errorHandler.handleError.calls.argsFor(0)[0].message).toEqual(
285283
'Loading icon set URL: farm-set-1.svg failed: Http failure response ' +
286284
'for farm-set-1.svg: 0 ');
287-
expect(errorHandler.handleError.calls.argsFor(1)[0].message)
288-
.toEqual(
289-
`Error retrieving icon ${testComponent.iconName}! ` +
290-
'Unable to find icon with the name "pig"');
291-
});
292-
293-
it('should delegate an error getting an SVG icon to the ErrorHandler', () => {
294-
iconRegistry.addSvgIconSetInNamespace('farm', trustUrl('farm-set-1.svg'));
295-
296-
const fixture = TestBed.createComponent(IconFromSvgName);
297-
const testComponent = fixture.componentInstance;
298-
299-
testComponent.iconName = 'farm:DNE';
300-
fixture.detectChanges();
301-
http.expectOne('farm-set-1.svg').flush(FAKE_SVGS.farmSet1);
302-
fixture.detectChanges();
303-
304-
// The HTTP request succeeded but the icon was not found so we logged.
305-
expect(errorHandler.handleError).toHaveBeenCalledTimes(1);
306-
expect(errorHandler.handleError.calls.argsFor(0)[0].message)
307-
.toEqual(
308-
`Error retrieving icon ${testComponent.iconName}! ` +
309-
'Unable to find icon with the name "DNE"');
310285
});
311286

312287
it('should extract icon from SVG icon set', () => {

src/material/icon/icon.ts

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,27 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {coerceBooleanProperty} from '@angular/cdk/coercion';
10-
import {DOCUMENT} from '@angular/common';
9+
import {take} from 'rxjs/operators';
1110
import {
12-
AfterViewChecked,
1311
Attribute,
1412
ChangeDetectionStrategy,
1513
Component,
1614
ElementRef,
17-
ErrorHandler,
18-
inject,
19-
Inject,
20-
InjectionToken,
2115
Input,
2216
OnChanges,
23-
OnDestroy,
2417
OnInit,
25-
Optional,
2618
SimpleChanges,
2719
ViewEncapsulation,
20+
Optional,
21+
InjectionToken,
22+
inject,
23+
Inject,
24+
OnDestroy,
25+
AfterViewChecked,
2826
} from '@angular/core';
27+
import {DOCUMENT} from '@angular/common';
2928
import {CanColor, CanColorCtor, mixinColor} from '@angular/material/core';
30-
import {take} from 'rxjs/operators';
31-
29+
import {coerceBooleanProperty} from '@angular/cdk/coercion';
3230
import {MatIconRegistry} from './icon-registry';
3331

3432

@@ -180,15 +178,14 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Aft
180178
private _elementsWithExternalReferences?: Map<Element, {name: string, value: string}[]>;
181179

182180
constructor(
183-
elementRef: ElementRef<HTMLElement>, private _iconRegistry: MatIconRegistry,
181+
elementRef: ElementRef<HTMLElement>,
182+
private _iconRegistry: MatIconRegistry,
184183
@Attribute('aria-hidden') ariaHidden: string,
185184
/**
186185
* @deprecated `location` parameter to be made required.
187186
* @breaking-change 8.0.0
188187
*/
189-
@Optional() @Inject(MAT_ICON_LOCATION) private _location?: MatIconLocation,
190-
// @breaking-change 9.0.0 _errorHandler parameter to be made required
191-
@Optional() private readonly _errorHandler?: ErrorHandler) {
188+
@Optional() @Inject(MAT_ICON_LOCATION) private _location?: MatIconLocation) {
192189
super(elementRef);
193190

194191
// 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
231228
if (this.svgIcon) {
232229
const [namespace, iconName] = this._splitIconName(this.svgIcon);
233230

234-
this._iconRegistry.getNamedSvgIcon(iconName, namespace)
235-
.pipe(take(1))
236-
.subscribe(svg => this._setSvgElement(svg), (err: Error) => {
237-
const errorMessage = `Error retrieving icon ${namespace}:${iconName}! ${err.message}`;
238-
// @breaking-change 9.0.0 _errorHandler parameter to be made required.
239-
if (this._errorHandler) {
240-
this._errorHandler.handleError(new Error(errorMessage));
241-
} else {
242-
console.error(errorMessage);
243-
}
244-
});
231+
this._iconRegistry.getNamedSvgIcon(iconName, namespace).pipe(take(1)).subscribe(
232+
svg => this._setSvgElement(svg),
233+
(err: Error) => console.log(`Error retrieving icon: ${err.message}`)
234+
);
245235
} else if (svgIconChanges.previousValue) {
246236
this._clearSvgElement();
247237
}

tools/public_api_guard/material/icon.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export declare class MatIcon extends _MatIconMixinBase implements OnChanges, OnI
2828
inline: boolean;
2929
svgIcon: string;
3030
constructor(elementRef: ElementRef<HTMLElement>, _iconRegistry: MatIconRegistry, ariaHidden: string,
31-
_location?: MatIconLocation | undefined, _errorHandler?: ErrorHandler | undefined);
31+
_location?: MatIconLocation | undefined);
3232
ngAfterViewChecked(): void;
3333
ngOnChanges(changes: SimpleChanges): void;
3434
ngOnDestroy(): void;

0 commit comments

Comments
 (0)