fix(icon): not taking current path after initialization #13641
Conversation
googlebot
added
the
cla: yes
jelbourn
added
pr: lgtm
action: merge
When resolving the current path while prefixing `url()` references, we use a provider to get the current path so we can account for server-side rendering and to be able to stub it out during tests. Since the provider is an object with a regular `pathname` property, the path will only be resolved once the first time an icon is resolved and then it'll be cached. This means that all subsequent icons will take an incorrect path. Related to angular#13628.
crisbeto
force-pushed
the
icon-outdated-path
branch
from
1ac9cd6 to
e18522d
Compare
| const elementsWithFuncIri = element.querySelectorAll(funcIriAttributeSelector); | ||
| const path = this._location.pathname ? this._location.pathname.split('#')[0] : ''; | ||
| const elements = this._elementsWithExternalReferences = |
There was a problem hiding this comment.
Why not just initialize _elementsWithExternalReferences as an empty Map?
There was a problem hiding this comment.
I did it so we can check for its presence in ngAfterViewChecked, rather than having to check the size. I'm not sure whether I'm not micro-optimizing it though.
| private _previousPath?: string; | ||
|
|
||
| /** Keeps track of the elements and attributes that we've prefixed with the current path. */ | ||
| private _elementsWithExternalReferences?: Map<Element, {name: string, value: string}[]>; |
There was a problem hiding this comment.
It looks like you reuse this as cachedElements in the ngAfterViewChecked method so that its a shorter name, should we just use cachedElements as the property name?
There was a problem hiding this comment.
I think that calling the property cachedElements is a little too generic.
|
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. |
When resolving the current path while prefixing
url()references, we use a provider to get the current path so we can account for server-side rendering and to be able to stub it out during tests. Since the provider is an object with a regularpathnameproperty, the path will only be resolved once the first time an icon is resolved and then it'll be cached. This means that all subsequent icons will take an incorrect path.Related to #13628.