🏗Multi-doc manager refactor#25629
Conversation
b264cdb to
b505c10
Compare
37758cf to
99780da
Compare
5597732 to
5da1154
Compare
jridgewell
left a comment
There was a problem hiding this comment.
If this works, awesome! Can you post the filesize diffs for v0.js and amp-next-page.js (compiled)?
| /** | ||
| * 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.
deb0c1d to
c9e9b73
Compare
c9e9b73 to
4266acf
Compare
|
Split the |
| 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.
| 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