Skip to content

perf(icon): avoid unnecessarily parsing icon sets #18654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 13, 2020
Merged

perf(icon): avoid unnecessarily parsing icon sets #18654

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 52 additions & 62 deletions src/material/icon/icon-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,17 @@ export interface IconOptions {
* @docs-private
*/
class SvgIconConfig {
url: SafeResourceUrl | null;
svgElement: SVGElement | null;

constructor(url: SafeResourceUrl, options?: IconOptions);
constructor(svgElement: SVGElement, options?: IconOptions);
constructor(data: SafeResourceUrl | SVGElement, public options?: IconOptions) {
// Note that we can't use `instanceof SVGElement` here,
// because it'll break during server-side rendering.
if (!!(data as any).nodeName) {
this.svgElement = data as SVGElement;
} else {
this.url = data as SafeResourceUrl;
}
}
constructor(
public url: SafeResourceUrl,
public svgText: string | null,
public options?: IconOptions) {}
}

/** Icon configuration whose content has already been loaded. */
type LoadedSvgIconConfig = SvgIconConfig & {svgText: string};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's cool to see how techniques like this develop and apply to code written when we were all first starting to use TypeScript.


/**
* Service to register and display icons used by the `<mat-icon>` component.
* - Registers icon URLs by namespace and name.
Expand Down Expand Up @@ -168,7 +163,7 @@ export class MatIconRegistry implements OnDestroy {
*/
addSvgIconInNamespace(namespace: string, iconName: string, url: SafeResourceUrl,
options?: IconOptions): this {
return this._addSvgIconConfig(namespace, iconName, new SvgIconConfig(url, options));
return this._addSvgIconConfig(namespace, iconName, new SvgIconConfig(url, null, options));
}

/**
Expand All @@ -179,14 +174,14 @@ export class MatIconRegistry implements OnDestroy {
*/
addSvgIconLiteralInNamespace(namespace: string, iconName: string, literal: SafeHtml,
options?: IconOptions): this {
const sanitizedLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);
const cleanLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);

if (!sanitizedLiteral) {
if (!cleanLiteral) {
throw getMatIconFailedToSanitizeLiteralError(literal);
}

const svgElement = this._createSvgElementForSingleIcon(sanitizedLiteral, options);
return this._addSvgIconConfig(namespace, iconName, new SvgIconConfig(svgElement, options));
return this._addSvgIconConfig(namespace, iconName,
new SvgIconConfig('', cleanLiteral, options));
}

/**
Expand All @@ -211,7 +206,7 @@ export class MatIconRegistry implements OnDestroy {
* @param url
*/
addSvgIconSetInNamespace(namespace: string, url: SafeResourceUrl, options?: IconOptions): this {
return this._addSvgIconSetConfig(namespace, new SvgIconConfig(url, options));
return this._addSvgIconSetConfig(namespace, new SvgIconConfig(url, null, options));
}

/**
Expand All @@ -221,14 +216,13 @@ export class MatIconRegistry implements OnDestroy {
*/
addSvgIconSetLiteralInNamespace(namespace: string, literal: SafeHtml,
options?: IconOptions): this {
const sanitizedLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);
const cleanLiteral = this._sanitizer.sanitize(SecurityContext.HTML, literal);

if (!sanitizedLiteral) {
if (!cleanLiteral) {
throw getMatIconFailedToSanitizeLiteralError(literal);
}

const svgElement = this._svgElementFromString(sanitizedLiteral);
return this._addSvgIconSetConfig(namespace, new SvgIconConfig(svgElement, options));
return this._addSvgIconSetConfig(namespace, new SvgIconConfig('', cleanLiteral, options));
}

/**
Expand Down Expand Up @@ -292,7 +286,7 @@ export class MatIconRegistry implements OnDestroy {
return observableOf(cloneSvg(cachedIcon));
}

return this._loadSvgIconFromConfig(new SvgIconConfig(safeUrl)).pipe(
return this._loadSvgIconFromConfig(new SvgIconConfig(safeUrl, null)).pipe(
tap(svg => this._cachedIconsByUrl.set(url!, svg)),
map(svg => cloneSvg(svg)),
);
Expand Down Expand Up @@ -335,15 +329,12 @@ export class MatIconRegistry implements OnDestroy {
* Returns the cached icon for a SvgIconConfig if available, or fetches it from its URL if not.
*/
private _getSvgFromConfig(config: SvgIconConfig): Observable<SVGElement> {
if (config.svgElement) {
if (config.svgText) {
// We already have the SVG element for this icon, return a copy.
return observableOf(cloneSvg(config.svgElement));
return observableOf(cloneSvg(this._svgElementFromConfig(config as LoadedSvgIconConfig)));
} else {
// Fetch the icon from the config's URL, cache it, and return a copy.
return this._loadSvgIconFromConfig(config).pipe(
tap(svg => config.svgElement = svg),
map(svg => cloneSvg(svg)),
);
return this._loadSvgIconFromConfig(config).pipe(map(svg => cloneSvg(svg)));
}
}

Expand All @@ -370,11 +361,11 @@ export class MatIconRegistry implements OnDestroy {

// Not found in any cached icon sets. If there are icon sets with URLs that we haven't
// fetched, fetch them now and look for iconName in the results.
const iconSetFetchRequests: Observable<SVGElement | null>[] = iconSetConfigs
.filter(iconSetConfig => !iconSetConfig.svgElement)
const iconSetFetchRequests: Observable<string | null>[] = iconSetConfigs
.filter(iconSetConfig => !iconSetConfig.svgText)
.map(iconSetConfig => {
return this._loadSvgIconSetFromConfig(iconSetConfig).pipe(
catchError((err: HttpErrorResponse): Observable<SVGElement | null> => {
catchError((err: HttpErrorResponse) => {
const url = this._sanitizer.sanitize(SecurityContext.RESOURCE_URL, iconSetConfig.url);

// Swallow errors fetching individual URLs so the
Expand Down Expand Up @@ -414,8 +405,14 @@ export class MatIconRegistry implements OnDestroy {
// Iterate backwards, so icon sets added later have precedence.
for (let i = iconSetConfigs.length - 1; i >= 0; i--) {
const config = iconSetConfigs[i];
if (config.svgElement) {
const foundIcon = this._extractSvgIconFromSet(config.svgElement, iconName, config.options);

// Parsing the icon set's text into an SVG element can be expensive. We can avoid some of
// the parsing by doing a quick check using `indexOf` to see if there's any chance for the
// icon to be in the set. This won't be 100% accurate, but it should help us avoid at least
// some of the parsing.
if (config.svgText && config.svgText.indexOf(iconName) > -1) {
const svg = this._svgElementFromConfig(config as LoadedSvgIconConfig);
const foundIcon = this._extractSvgIconFromSet(svg, iconName, config.options);
if (foundIcon) {
return foundIcon;
}
Expand All @@ -429,38 +426,22 @@ export class MatIconRegistry implements OnDestroy {
* from it.
*/
private _loadSvgIconFromConfig(config: SvgIconConfig): Observable<SVGElement> {
return this._fetchIcon(config)
.pipe(map(svgText => this._createSvgElementForSingleIcon(svgText, config.options)));
return this._fetchIcon(config).pipe(
tap(svgText => config.svgText = svgText),
map(() => this._svgElementFromConfig(config as LoadedSvgIconConfig))
);
}

/**
* Loads the content of the icon set URL specified in the SvgIconConfig and creates an SVG element
* from it.
* Loads the content of the icon set URL specified in the
* SvgIconConfig and attaches it to the config.
*/
private _loadSvgIconSetFromConfig(config: SvgIconConfig): Observable<SVGElement> {
// If the SVG for this icon set has already been parsed, do nothing.
if (config.svgElement) {
return observableOf(config.svgElement);
private _loadSvgIconSetFromConfig(config: SvgIconConfig): Observable<string | null> {
if (config.svgText) {
return observableOf(null);
}

return this._fetchIcon(config).pipe(map(svgText => {
// It is possible that the icon set was parsed and cached by an earlier request, so parsing
// only needs to occur if the cache is yet unset.
if (!config.svgElement) {
config.svgElement = this._svgElementFromString(svgText);
}

return config.svgElement;
}));
}

/**
* Creates a DOM element from the given SVG string, and adds default attributes.
*/
private _createSvgElementForSingleIcon(responseText: string, options?: IconOptions): SVGElement {
const svg = this._svgElementFromString(responseText);
this._setSvgAttributes(svg, options);
return svg;
return this._fetchIcon(config).pipe(tap(svgText => config.svgText = svgText));
}

/**
Expand Down Expand Up @@ -596,8 +577,6 @@ export class MatIconRegistry implements OnDestroy {
return inProgressFetch;
}

// TODO(jelbourn): for some reason, the `finalize` operator "loses" the generic type on the
// Observable. Figure out why and fix it.
const req = this._httpClient.get(url, {responseType: 'text', withCredentials}).pipe(
finalize(() => this._inProgressUrlFetches.delete(url)),
share(),
Expand Down Expand Up @@ -634,6 +613,17 @@ export class MatIconRegistry implements OnDestroy {

return this;
}

/** Parses a config's text into an SVG element. */
private _svgElementFromConfig(config: LoadedSvgIconConfig): SVGElement {
if (!config.svgElement) {
const svg = this._svgElementFromString(config.svgText);
this._setSvgAttributes(svg, config.options);
config.svgElement = svg;
}

return config.svgElement;
}
}

/** @docs-private */
Expand Down