Skip to content
This repository has been archived by the owner on May 17, 2019. It is now read-only.

Switch to parser-inserted scripts, fix script preloading for legacy bundles #612

Merged
merged 1 commit into from
Nov 8, 2018

Conversation

rtsao
Copy link
Member

@rtsao rtsao commented Nov 8, 2018

Summary

  • Fixes script preloading in legacy browsers

  • Replaces dynamic script loading with scripts in markup to ensure the scripts are marked as parser-inserted. Not all browsers respect defer attributes for scripts that are not marked as parser-inserted. This caused problems in FF/Edge. https://html.spec.whatwg.org/multipage/scripting.html

    • Replace runtime Edge UA check with server-side UA check (to avoid bugs with native classes)
    • Replace runtime nomodule support check with server-side UA check for Safari 10.1 (to prevent double script download/execution)

@rtsao rtsao added the bugfix label Nov 8, 2018
@rtsao rtsao force-pushed the parser-inserted-scripts branch 2 times, most recently from b3ae2b2 to a2d3344 Compare November 8, 2018 21:14
@rtsao
Copy link
Member Author

rtsao commented Nov 8, 2018

!merge

@old-fusion-bot old-fusion-bot bot merged commit 3ca0b69 into master Nov 8, 2018
browser.version.startsWith('10.1');

// Safari 10.1 supports modules but not `nomodule` attribute.
if (!isSafari10_1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be reading this wrong but it looks like non-edge, non-safari 10.1 browsers get both modern and legacy bundles in criticalChunkScripts

Copy link
Member Author

@rtsao rtsao Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's exactly as intended.

Browsers that do not support <script type="module"> will simply ignore them. And all browsers that support <script type="module"> will ignore <script nomodule> scripts, except for Safari 10.1, which will ignore nomodule thereby downloading and executing both type="module" and nomodule scripts. So exclusively for Safari 10.1, only <script type="module"> should be sent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@KevinGrandon KevinGrandon deleted the parser-inserted-scripts branch November 8, 2018 21:52
@AlexMSmithCA AlexMSmithCA mentioned this pull request Nov 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants