remove cached owner info #6028
remove cached owner info #6028
Conversation
| @@ -103,6 +104,15 @@ export class Resource { | |||
| Resource.forElementOptional(element).updateOwner(owner); | |||
| } | |||
| element[OWNER_PROP_] = owner; | |||
| // Need to clear owner cache for all child elements | |||
There was a problem hiding this comment.
We definitely want this to be recursive, as it could be <owner> <child> <div> <grandchild>. Using Resources#discoverResourcesForElement_ or #discoverResourcesForArray_ would do it.
There was a problem hiding this comment.
Good point, it needs to be recursive. However I don't think Resources#discoverResourcesForElement_ or #discoverResourcesForArray_ will do the work here. In the case of <owner> <child> <div> <grandchild> #discoverResourcesForElement_ will stop searching when it reaches <child>. Thus will never clear the cached value for <grandchild>. Also what if <owner><child><div><grandchild><grandgrandchild>?
A correct approach I can think of would be finding all the children, grandchildren .... (sorry I'm too lazy find the opposite word for ancestor 😄), but I assume it would be time consuming.
There was a problem hiding this comment.
I switched to getElementByClassName('-amp-element') which finds me all the amp children.
It's same concept as discoverResourcesForElement_ except discoverResourcesForElement doesn't work in case <owner><child><grandchild><greatgrandchild> because it will not clear cache for greatgrandchild.
PTAL
| @@ -190,7 +200,7 @@ export class Resource { | |||
|
|
|||
| /** | |||
| * Update owner element | |||
| * @param {!AmpElement} owner | |||
| * @param {AmpElement|undefined} owner | |||
There was a problem hiding this comment.
How would this be undefined?
There was a problem hiding this comment.
when I want to clear the cached value and reset it to undefine.
* owner * use getElementsByClassName
* owner * use getElementsByClassName
fix #5917