🏗Multi-doc manager refactor #25629
🏗Multi-doc manager refactor #25629
Conversation
wassgha
force-pushed
the
dynamic-shadow-mode
branch
2 times, most recently
from
b264cdb to
b505c10
Compare
wassgha
force-pushed
the
dynamic-shadow-mode
branch
from
37758cf to
99780da
Compare
wassgha
changed the title
wassgha
force-pushed
the
dynamic-shadow-mode
branch
from
5597732 to
5da1154
Compare
wassgha
requested review from
jridgewell
and removed request for
dreamofabear
| /** | ||
| * A manager for documents in the multi-doc environment. | ||
| */ | ||
| export class MultidocManager { |
| dev().fine(TAG, 'Attach to shadow root:', shadowRoot, ampdoc); | ||
|
|
||
| // Install runtime CSS. | ||
| installStylesForDoc( |
There was a problem hiding this comment.
Doing a diff, these two lines seem to be the only changes? I'm astounded that this was all that's needed.
There was a problem hiding this comment.
Yup! I essentially did a diff of multidoc-manager dependency tree and runtime's dependency tree to see what multidoc-manager needs that's not provided by runtime. I'll do more testing to make sure this doesn't break anything, otherwise I'll just exclude the inabox runtime from this change and we should be good to go.
wassgha
force-pushed
the
dynamic-shadow-mode
branch
from
deb0c1d to
c9e9b73
Compare
wassgha
force-pushed
the
dynamic-shadow-mode
branch
from
c9e9b73 to
4266acf
Compare
|
Split the |
| /** | ||
| * Applies the runtime to a given global scope for shadow mode. | ||
| * @param {!Window} global Global scope to adopt. | ||
| * @return {!Promise} | ||
| */ | ||
| export function adoptShadowMode(global) { | ||
| return adoptShared(global, (global, extensions) => { | ||
| // shadow mode already adopted | ||
| if (global.AMP.attachShadowDoc) { |
There was a problem hiding this comment.
Hmm not sure I understand, it's set under it on line 426
There was a problem hiding this comment.
Ahh, that's not part of Github's diff display.
| /** | ||
| * Applies the runtime to a given global scope for shadow mode. | ||
| * @param {!Window} global Global scope to adopt. | ||
| * @return {!Promise} | ||
| */ | ||
| export function adoptShadowMode(global) { | ||
| return adoptShared(global, (global, extensions) => { | ||
| // shadow mode already adopted | ||
| if (global.AMP.attachShadowDoc) { |
There was a problem hiding this comment.
Ahh, that's not part of Github's diff display.
* Separates multidoc-manager from the runtime * Export installAmpdocServices * Implements dynamic adoption of shadow mode * Type check * Fix type checking * Revert dynamic adoption * Export css dependency from multidoc-manager * Removed dynamic asdoption of shadow mode * Excludes inabox and video-iframe-integration from requiring Multidoc dependencies * Type check fix
Closes #25630 , partial for #25500
Changes
MultidocManagerfromruntime.jsMultidocManagerrequires and injects them through theglobal.AMPobject (only for thev0.jsand shadow-v0 bundles, excludesinabox/amp4adsandvideo-iframe-integration. Biggest gain from this is a smalleramp-next-pagebundle size with no significant runtime size increase.Bundle size results
dist/amp4ads-v0.js: Δ -0.08KBdist/shadow-v0.js: Δ +0.02KBdist/v0.js: Δ -0.03KBdist/v0/amp-next-page-0.1.js: Δ -38.44KB