Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use nomodule to conditionally load fetch+promise #833

Merged
merged 1 commit into from Jul 10, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -23,17 +23,17 @@
<%= htmlWebpackPlugin.options.ssr() %>
<% if (webpack.assets.filter(entry => entry.name.match(/bundle(\.\w{5})?.esm.js$/)).length > 0) { %>
<% /* Fix for safari < 11 nomodule bug. TODO: Do the following only for safari. */ %>
<script>!function(){var e=document,t=e.createElement("script");if(!("noModule"in t)&&"onbeforeload"in t){var n=!1;e.addEventListener("beforeload",function(e){if(e.target===t)n=!0;else if(!e.target.hasAttribute("nomodule")||!n)return;e.preventDefault()},!0),t.type="module",t.src=".",e.head.appendChild(t),t.remove()}}();</script>
<script nomodule>!function(){var e=document,t=e.createElement("script");if(!("noModule"in t)&&"onbeforeload"in t){var n=!1;e.addEventListener("beforeload",function(e){if(e.target===t)n=!0;else if(!e.target.hasAttribute("nomodule")||!n)return;e.preventDefault()},!0),t.type="module",t.src=".",e.head.appendChild(t),t.remove()}}();</script>
<script crossorigin="anonymous" src="<%= htmlWebpackPlugin.files.publicPath %><%= webpack.assets.filter(entry => entry.name.match(/bundle(\.\w{5})?.esm.js$/))[0].name %>" type="module"></script>
<%
/*Fetch and Promise polyfills are not needed for browsers that support type=module
Please re-evaluate below line if adding more polyfills.*/
%>
<script nomodule>window.fetch||document.write('<script src="<%= htmlWebpackPlugin.files.chunks["polyfills"].entry %>"><\/script>')</script>
<script nomodule src="<%= htmlWebpackPlugin.files.chunks["polyfills"].entry %>"></script>
<script nomodule defer src="<%= htmlWebpackPlugin.files.chunks['bundle'].entry %>"></script>
<% } else { %>
<script <%= htmlWebpackPlugin.options.scriptLoading %> src="<%= htmlWebpackPlugin.files.chunks['bundle'].entry %>"></script>
<script>window.fetch||document.write('<script src="<%= htmlWebpackPlugin.files.chunks["polyfills"].entry %>"><\/script>')</script>
<script nomodule src="<%= htmlWebpackPlugin.files.chunks["polyfills"].entry %>"></script>

This comment has been minimized.

Copy link
@prateekbh

prateekbh Jul 10, 2019

Member

Why does this need nomodule?
This section only executes if we --no-esm is used. So i guess its better with window.fetch||document.write?

This comment has been minimized.

Copy link
@ForsakenHarmony

ForsakenHarmony Jul 10, 2019

Member

it still works though and no javascript has to be executed for it?

This comment has been minimized.

Copy link
@prateekbh

prateekbh Jul 10, 2019

Member

yep but if someone is explicitly telling you, they don't want --esm than this just worsens the performance for them. WDYT?

This comment has been minimized.

Copy link
@ForsakenHarmony

ForsakenHarmony Jul 10, 2019

Member

it worsens performance?

This comment has been minimized.

Copy link
@prateekbh

prateekbh Jul 10, 2019

Member

Like the nomodule compatible browsers might not need the polyfills but this will still give it to them.
Or am i missing something?

This comment has been minimized.

Copy link
@ForsakenHarmony

ForsakenHarmony Jul 10, 2019

Member

This switches from using document.write() for polyfill injection to using <script nomodule>. It's not a perfect match, so it'll be loading fetch+Promise in Safari 10 where they're natively supported, but the performance benefits in other browsers seem worthwhile.

This comment has been minimized.

Copy link
@prateekbh

prateekbh Jul 10, 2019

Member

/me dumb.
/me hides in a corner

This comment has been minimized.

Copy link
@prateekbh

prateekbh Jul 10, 2019

Member

sorry didn't read it through my bad totally

<% } %>
</body>
</html>
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.