fix(icon): prevent parsing the same icon set multiple times #9635
Conversation
jelbourn
added
P1
googlebot
added
the
cla: yes
This fixes an issue where requests for icons in a particular set made while that set is being fetched result in the set being parsed into an SVGElement multiple times. It occured because, while the same pending http request was used for each icon, each icon added their own `map` onto that request, will every `map` resulting in a (potentially expensive) parse. This change sets the cached icon earlier and checks it later.
jelbourn
force-pushed
the
icon-set-caching
branch
from
652d501 to
c02f878
Compare
mmalerba
added
pr: lgtm
action: merge
| // Normally we avoid spying on private methods like this, but the parsing is a private | ||
| // implementation detail that should not be exposed to the public API. This test, though, | ||
| // is important enough to warrant the brittle-ness that results. | ||
| spyOn(iconRegistry, '_svgElementFromString' as any).and.callThrough(); |
There was a problem hiding this comment.
Rather than spying on a private method, you should be able to assert the same by reproviding the DOCUMENT token and spying on createElement.
There was a problem hiding this comment.
I think long-term the "right" thing to do is to break SVG parsing into its own utility and then use a mock for MatIconRegistry, which would make sense to do as part of #5188
| // 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); |
There was a problem hiding this comment.
This one could be shortened to config.svgElement = config.svgElement || this._svgElementFromString(svgText).
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This fixes an issue where requests for icons in a particular set made while that set is being fetched result in the set being parsed into an SVGElement multiple times. It occured because, while the same pending http request was used for each icon, each icon added their own
maponto that request, will everymapresulting in a (potentially expensive) parse. This change sets the cached icon earlier and checks it later.