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

fix(index): dynamic css loading support for older browsers #134

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 33 additions & 2 deletions src/index.js
Expand Up @@ -340,7 +340,20 @@ class MiniCssExtractPlugin {
'var linkTag = document.createElement("link");',
'linkTag.rel = "stylesheet";',
'linkTag.type = "text/css";',
'linkTag.onload = resolve;',
'linkTag.href = fullhref;',
"// old webkit's would claim to have onload, but didn't really support it",
'// https://github.com/kriszyp/xstyle/blob/master/core/load-css.js',
'var webkitVersion = navigator.userAgent.match(/AppleWebKit\\/(\\d+\\.?\\d*)/);',
Copy link
Member

Choose a reason for hiding this comment

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

This extra check affects only 0.02% of the internet. I don't think it's worth including this workaround of everybody.

'webkitVersion = webkitVersion && +webkitVersion[1];',
'if(linkTag.onload === null && !(webkitVersion < 536)){',
'// most browsers support this onload function now',
'linkTag.onload = function(){',
'// cleanup',
'linkTag.onload = null;',
Copy link
Member

Choose a reason for hiding this comment

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

indent

'linkTag.onerror = null;',
'resolve();',
'};',
'// always add the error handler, so we can notify of any errors',
'linkTag.onerror = function(event) {',
Template.indent([
'var request = event && event.target && event.target.src || fullhref;',
Expand All @@ -349,7 +362,25 @@ class MiniCssExtractPlugin {
'reject(err);',
]),
'};',
'linkTag.href = fullhref;',
'} else {',
'var errorTimeout = 60000;',
Copy link
Member

Choose a reason for hiding this comment

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

What about is loading more than 60000 ?

Copy link
Author

Choose a reason for hiding this comment

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

It's a timeout threshold for css loading. To make sure chunk is resolved, I think we need a stop time to clear setInterval when the css loaded fail.

'var startTime = Date.now();',
'var interval = setInterval(function(){',
'if(linkTag.style){',
Copy link
Member

Choose a reason for hiding this comment

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

Instead of polling this seem to be a better solution for this problem: https://github.com/webpack-contrib/mini-css-extract-plugin/pull/299/files

'clearInterval(interval);',
Copy link
Member

Choose a reason for hiding this comment

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

indent

'resolve();',
'}',
'if(!linkTag.style && (Date.now() - startTime) > errorTimeout){',
Copy link
Member

Choose a reason for hiding this comment

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

The support for onload and onerror differ. Make sure to handle them separately. Use a setTimeout for error handling and don't merge it into the setInterval for onload.

Template.indent([
'var request = fullhref;',
'var err = new Error("Loading CSS chunk " + chunkId + " timeout in old browser.\\n(" + request + ")");',
'err.request = request;',
'clearInterval(interval);',
'reject(err);',
]),
'}',
'}, 25);',
'}',
'var head = document.getElementsByTagName("head")[0];',
'head.appendChild(linkTag);',
]),
Expand Down