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 2 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
4 changes: 4 additions & 0 deletions src/index.js
Expand Up @@ -338,8 +338,10 @@ class MiniCssExtractPlugin {
]),
'}',
'var linkTag = document.createElement("link");',
'var isOldWebKit = Number(navigator.userAgent.replace(/.*AppleWebKit\\/(\\d+)..*/, "$1")) < 536;',
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can find better solution for check this, can you investigate how this do example jquery or other frontend libraries?

Copy link
Author

Choose a reason for hiding this comment

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

At present, old browser is less. I only use regex to match AppleWebKit 536, bescause I think it will cover most of old browser. There is another solution to check version:
https://github.com/filamentgroup/loadCSS/blob/master/src/onloadCSS.js
As bellow is another solution to check if css is loaded:
http://www.backalleycoder.com/2011/03/20/link-tag-css-stylesheet-load-event/
https://www.zachleat.com/web/load-css-dynamically/
Another question: Why we need to wait resolve for css chunks is loaded? CSS chunkses are appended in sequence, they can't impact each other. The waiting will increase js render time inversely. There is so less case we need to caculate element style when the the self css is loaded but next css is not loaded.

Copy link
Member

Choose a reason for hiding this comment

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

@gaterking http://www.backalleycoder.com/2011/03/20/link-tag-css-stylesheet-load-event/ looks universal, but create new dom node looks overload.

Give me time on thinking about best solution, ping be tomorrow and we find better.

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/guybedford/require-css/blob/master/css.js
A CSS Loader plugin for RequireJS which can be refered to.

Copy link
Member

Choose a reason for hiding this comment

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

@gaterking looks solid, maybe we can port solution here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it's solid. We can remove some inspections such as ie or low version firefox. Webpack 4 don't support ie6 to 8.

Copy link
Member

Choose a reason for hiding this comment

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

@gaterking we can avoid part of code for old ie

'linkTag.rel = "stylesheet";',
'linkTag.type = "text/css";',
'if (!isOldWebKit) {',
'linkTag.onload = resolve;',
'linkTag.onerror = function(event) {',
Template.indent([
Expand All @@ -349,9 +351,11 @@ class MiniCssExtractPlugin {
'reject(err);',
]),
'};',
'}',
'linkTag.href = fullhref;',
'var head = document.getElementsByTagName("head")[0];',
'head.appendChild(linkTag);',
'if (isOldWebKit) { resolve();}',
]),
'}).then(function() {',
Template.indent(['installedCssChunks[chunkId] = 0;']),
Expand Down