Exposing meta tags from attachShadowDoc #22735
Conversation
|
@dvoytenko if the approach looks fine, i'll go ahead and add tests around this |
src/runtime.js
Outdated
| @@ -734,13 +734,13 @@ export class MultidocManager { | |||
| dev().fine(TAG, '- set title: ', shadowRoot.AMP.title); | |||
| break; | |||
| case 'META': | |||
| shadowRoot.AMP.meta = shadowRoot.AMP.meta || []; | |||
There was a problem hiding this comment.
I really feel like we should just simply do:
shadowRoot.AMP.head = doc.head
This would let publishers to just do head.querySelector('meta[name=X]') and all that stuff.
There was a problem hiding this comment.
nit: I see that [shadowRoot.ampdoc.getHeadNode().children]( https://github.com/ampproject/amphtml/issues/22587#issuecomment-497877987) gives a filtered subset of the actual head.
Would this create like a conflict with the same? Or create confusion for publishers as to choose which one?
There was a problem hiding this comment.
It shouldn't. Because the ampdoc is not really in the user-space APIs - it's intended for the runtime uses specifically. And likely will be obfuscated soon. The returned AMP object, however, we can shape as we want.
There was a problem hiding this comment.
I'm trying to understand why this is different from the next case for "LINK" tags where they are automatically appended to the document's head.
Can it do the same for meta tags or is there a reason why not to do it from here?
There was a problem hiding this comment.
I really feel like we should just simply do:
shadowRoot.AMP.head = doc.headThis would let publishers to just do
head.querySelector('meta[name=X]')and all that stuff.
@dvoytenko regarding your suggestion, publishers can currently do exactly this by querying the original doc before passing it to attachShadowDoc so I don't see the big enhancement by adding it to the AMP object unless for future updates to remove the requirement of passing a doc (AMP will make the call. Wishful thinking :)).
So, I think it will be more useful to do the full job by appending it directly to head (see my other comment) or attaching it as an array so no need to query anything.
But, as said, it's really up to you guys to decide.
There was a problem hiding this comment.
@Joelsz the big benefit should come from attachShadowDocAsStream, where the document is streamed.
prateekbh
changed the title
* exposing meta tags from attachShadowDoc * exposes doc head via attachDoc in shadow mode * removed copying meta
fixes #22587