bug(icon): custom directive cannot set svgIcon for mat-icon

[klemenoslaj]

Reproduction

svgIcon does not seem to work with Stackblitz, providing repo instead.

Steps to reproduce:

1. Clone https://github.com/klemenoslaj/mat-icon-svg-not-applied
2. Install dependencies using yarn
3. Execute ng serve

Expected Behavior

I tried to create a custom directive that would programmatically assign some icon name to the svgIcon input of mat-icon, depending on some global icon mapping configuration.
This normally works for any component and I'd expect it should for mat-icon too.

Example:

<mat-icon [iconFor]="object"></mat-icon>

@Directive({
  selector: 'mat-icon[iconFor]',
})
export class IconForDirective {
  @Input()
  set iconFor(obj: SomeInterface) {
    this._matIcon.svgIcon = _iconMap[obj.type];
  }

  constructor(
    @Inject(ICON_MAPPING_TOKEN)
    private readonly _iconMap: SomeIconMapping,
    private readonly _matIcon: MatIcon,
  ) {}
}

Actual Behavior

The behavior described above does not work with mat-icon, because the relevant logic is in ngOnChanges hook of mat-icon, which does not get triggered, unless at least one Input is present on the host component.

components/src/material/icon/icon.ts

Lines 229 to 264 in 4f1238f

	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();
	}
	}

Workaround (ugly):

Instead of

<mat-icon [iconFor]="object"></mat-icon>

add dummy/empty svgIcon

<mat-icon [iconFor]="object" svgIcon></mat-icon>

Environment

• Angular: 10.0.14
• CDK/Material: 10.1.3
• Browser(s): any
• Operating System (e.g. Windows, macOS, Ubuntu): any

---

NOTE: I can submit the PR for this, but would like some thoughts on that, maybe it's expected like this for some reason.

[jelbourn]

Looking at the code, it seems like we should extract the code in ngOnChanges that actually performs the DOM update into its own function, and then call that from both ngOnChanges and ngOnInit if the value has been changed. @crisbeto thoughts on this since you've touched the icon code most recently?

[crisbeto]

The only way would be to define a setter for svgIcon. I know that in the past we've decided not to handle cases like this, but in this one I think it's fine since we're already "paying" for the setters on fontIcon and fontSet. I've submitted a PR to support it and we can discuss any concerns there (#20509).

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}