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

gaterking · 2018-05-08T08:30:16Z

This PR contains a:

Motivation / Use-Case

Breaking Changes

Additional Info

In old browser, such as android 4.3, link tag not support 4.3. Although these devices is few, the onload callback resolve function will not be called. This PR ignore onload for these devices and resolve immediately.

jsf-clabot · 2018-05-08T08:30:18Z

All committers have signed the CLA.

alexander-akait · 2018-05-08T09:27:03Z

@gaterking can you provide minimum reproducible test repo and browser version to ensure it is does not works for all, thanks!

gaterking · 2018-05-08T10:10:49Z

@evilebottnawi http://pie.gd/test/script-link-events/
From this test page, we can see many old browsers not support link tag onload event.

alexander-akait · 2018-05-08T10:13:18Z

@gaterking also please accept CLA

alexander-akait · 2018-05-08T10:14:57Z

@gaterking can you faced this problem ?

alexander-akait · 2018-05-08T10:21:49Z

src/index.js

@@ -338,8 +338,10 @@ class MiniCssExtractPlugin {
                  ]),
                  '}',
                  'var linkTag = document.createElement("link");',
+                  'var isOldWebKit = Number(navigator.userAgent.replace(/.*AppleWebKit\\/(\\d+)..*/, "$1")) < 536;',


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

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.

@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.

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

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

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

@gaterking we can avoid part of code for old ie

gaterking · 2018-05-08T10:22:59Z

@evilebottnawi Yes, I faced it in my vue project. Android 4.3 browser will fail, bescause some lazy load modules with css chunk not resolve.

alexander-akait · 2018-05-08T10:32:31Z

@gaterking looks good solution, but i think we should search better solution for detecting this

Jessidhia · 2018-05-09T07:37:37Z

This also happened to be a blocker for a smartphone-specific project at work. I'll link this to the person involved so he can check if this works 🤔

Jessidhia · 2018-05-09T07:40:23Z

Note that it's important to know when a dynamically inserted style is loaded. Not knowing this leads to FOUC, which can be disastrous if you're doing things like getBoundingClientRect on component mount if it happens before the load is done.

gaterking · 2018-05-12T11:29:39Z

index.js
@evilebottnawi Can you review my new code? If OK, I will create a new PR.
Refer to RequieJS plugin, it use regex to check browser, but it don't check webkit version.
In new solution, I refer to Dojo plugin and xstyle. I use setInterval to check css file is loaded in this case.
I am sure we need to support webkit < 536 only, Becase of Webpack 4.

gaterking · 2018-05-16T15:46:38Z

what the next step for this PR?

alexander-akait · 2018-05-16T16:06:17Z

src/index.js

@@ -349,7 +362,25 @@ class MiniCssExtractPlugin {
                    'reject(err);',
                  ]),
                  '};',
-                  'linkTag.href = fullhref;',
+                  '} else {',
+                  'var errorTimeout = 60000;',


What about is loading more than 60000 ?

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.

hxlniada · 2018-08-23T08:32:09Z

may be this is a better solution: https://stackoverflow.com/a/5371426

alexander-akait · 2018-08-23T09:52:19Z

/cc @gaterking

gaterking · 2018-09-20T09:05:27Z

I think this PR is not necessary for most developer, maybe we can close it. If somebody need to support old browser, I suggest to clone own repo.

lili21 · 2018-10-16T05:17:19Z

@evilebottnawi what do you think?

lili21 · 2018-10-16T06:08:59Z

@hxlniada It's a neat solution, just need a little change.

...
var img = document.createElement('img');
img.onerror = function (e) {
  var request = event && event.target && event.target.src || fullhref;
  var i = document.styleSheets.length;
  while(i--) {
    if (document.styleSheets[i].href === request) {
      return resolve();
    }
  }
  var err = new Error('Loading CSS chunk ' + chunkId + ' failed.\n(' + request + ')');
  err.request = request;
  reject(err);
};
img.src = fullhref;
...

gaterking · 2018-11-14T13:37:39Z

#299 similar PR

sokra · 2018-11-14T14:03:25Z

src/index.js

+                  '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*)/);',


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

sokra · 2018-11-14T14:04:10Z

src/index.js

+                  '// most browsers support this onload function now',
+                  'linkTag.onload = function(){',
+                  '// cleanup',
+                  'linkTag.onload = null;',


sokra · 2018-11-14T14:06:50Z

src/index.js

+                  'var startTime = Date.now();',
+                  'var interval = setInterval(function(){',
+                  'if(linkTag.style){',
+                  'clearInterval(interval);',


sokra · 2018-11-14T14:07:58Z

src/index.js

+                  'clearInterval(interval);',
+                  'resolve();',
+                  '}',
+                  'if(!linkTag.style && (Date.now() - startTime) > errorTimeout){',


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.

sokra · 2018-11-14T14:12:14Z

src/index.js

+                  'var errorTimeout = 60000;',
+                  'var startTime = Date.now();',
+                  'var interval = setInterval(function(){',
+                  'if(linkTag.style){',


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

codecov · 2020-05-03T16:02:34Z

Codecov Report

Merging #134 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #134   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           3       3           
  Lines         241     241           
  Branches       49      49           
======================================
  Misses        198     198           
  Partials       43      43

Impacted Files	Coverage Δ
src/index.js	`0.00% <ø> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41347f7...bd891a0. Read the comment docs.

gaterking added 2 commits May 8, 2018 09:53

feat: add css async load support to old webkit browser

79bde8c

fix: change old browser regex defined

7b2b070

alexander-akait reviewed May 8, 2018

View reviewed changes

alexander-akait added type: bug semver: patch priority: 2 (urgent) labels May 8, 2018

refactor: use setInterval to check style is loaded in old webkit browser

bd891a0

alexander-akait requested changes May 16, 2018

View reviewed changes

michael-ciniawsky changed the title ~~dynamic import css chunk support old browser~~ fix(index): dynamic import css chunk support older browsers Aug 6, 2018

michael-ciniawsky added this to the 0.4.2 milestone Aug 6, 2018

michael-ciniawsky changed the title ~~fix(index): dynamic import css chunk support older browsers~~ fix(index): dynamic css loading support for older browsers Aug 6, 2018

michael-ciniawsky added pr: review status: pending labels Aug 6, 2018

alexander-akait mentioned this pull request Oct 11, 2018

link element load event not fired on old browser. #294

Closed

sokra requested changes Nov 14, 2018

View reviewed changes

snyk-bot mentioned this pull request Jul 26, 2021

[Snyk] Upgrade schema-utils from 3.0.0 to 3.1.0 1Karmacoin/mini-css-extract-plugin#2

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

gaterking commented May 8, 2018

jsf-clabot commented May 8, 2018 •

edited

Loading

alexander-akait commented May 8, 2018

gaterking commented May 8, 2018

alexander-akait commented May 8, 2018

alexander-akait commented May 8, 2018

alexander-akait May 8, 2018

gaterking May 8, 2018

alexander-akait May 8, 2018

gaterking May 8, 2018

alexander-akait May 8, 2018

gaterking May 8, 2018

alexander-akait May 9, 2018

gaterking commented May 8, 2018

alexander-akait commented May 8, 2018

Jessidhia commented May 9, 2018

Jessidhia commented May 9, 2018

gaterking commented May 12, 2018

gaterking commented May 16, 2018

alexander-akait May 16, 2018

gaterking May 16, 2018

hxlniada commented Aug 23, 2018

alexander-akait commented Aug 23, 2018

gaterking commented Sep 20, 2018

lili21 commented Oct 16, 2018

lili21 commented Oct 16, 2018

gaterking commented Nov 14, 2018 •

edited

Loading

sokra Nov 14, 2018

sokra Nov 14, 2018

sokra Nov 14, 2018

sokra Nov 14, 2018

sokra Nov 14, 2018

codecov bot commented May 3, 2020 •

edited

Loading

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

Are you sure you want to change the base?

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

Conversation

gaterking commented May 8, 2018

Motivation / Use-Case

Breaking Changes

Additional Info

jsf-clabot commented May 8, 2018 • edited Loading

alexander-akait commented May 8, 2018

gaterking commented May 8, 2018

alexander-akait commented May 8, 2018

alexander-akait commented May 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaterking commented May 8, 2018

alexander-akait commented May 8, 2018

Jessidhia commented May 9, 2018

Jessidhia commented May 9, 2018

gaterking commented May 12, 2018

gaterking commented May 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hxlniada commented Aug 23, 2018

alexander-akait commented Aug 23, 2018

gaterking commented Sep 20, 2018

lili21 commented Oct 16, 2018

lili21 commented Oct 16, 2018

gaterking commented Nov 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 3, 2020 • edited Loading

Codecov Report

jsf-clabot commented May 8, 2018 •

edited

Loading

gaterking commented Nov 14, 2018 •

edited

Loading

codecov bot commented May 3, 2020 •

edited

Loading