3p-frame: Prefer #core/mode to getMode#35669
Conversation
| function mockMode(mode) { | ||
| env.sandbox.stub(window.parent, '__AMP_MODE').value(mode); | ||
| function mockMode(options) { | ||
| env.sandbox.stub(window.parent, '__AMP_MODE').value(options); |
There was a problem hiding this comment.
Is this line still necessary?
There was a problem hiding this comment.
This is the one call where removing the line breaks the test case:
amphtml/test/unit/3p/test-3p-frame.js
Lines 153 to 158 in 5ece445
It looks like this is specifically because the mode changes when not mocked via window:
amphtml/src/iframe-attributes.js
Line 90 in 7df04fa
I propose we leave the stub for now, and in a separate PR, see about updating getModeObject to reference #core/mode over getMode(win):
Lines 25 to 33 in 7df04fa
rcebulko
left a comment
There was a problem hiding this comment.
It looks like this is not being DCE'd correctly; the component bundle sizes are spiking. Taking a look now
rcebulko
left a comment
There was a problem hiding this comment.
I had seen ~150B increase in some bundles, but I locally built this PR and its merge base and compared the files directly to find no size diff. Looks like an artifact of CI using a different merge base or something similar. Re-approving
Thanks for looking into this! |
This change enables 3p iframe Bento components to reference the mode without having to be on an AMP document.