Skip to content
This repository has been archived by the owner on Oct 21, 2022. It is now read-only.

Network latency can cause css to block render in some cases. #19

Closed
scottjehl opened this issue Oct 23, 2014 · 10 comments · Fixed by #21
Closed

Network latency can cause css to block render in some cases. #19

scottjehl opened this issue Oct 23, 2014 · 10 comments · Fixed by #21
Assignees

Comments

@scottjehl
Copy link
Member

I noticed this when testing over 3g. Sometimes, our timeout is fast enough to toggle the link's media attribute to all before its stylesheet request goes out, and it ends up blocking render. Not good.

One workaround I've come up with so far involves polling the document.styleSheets array until it increments, which I've found is a safe time to toggle the media attribute on any connection speed, as the request appears to be in flight at the point it joins that array (yet it's still long before onload, which is good for minimizing the times this logic would have to run). That change looks something like this:

/*!
loadCSS: load a CSS file asynchronously.
[c]2014 @scottjehl, Filament Group, Inc.
Licensed MIT
*/
function loadCSS( href, before, media ){
    "use strict";
    // Arguments explained:
    // `href` is the URL for your CSS file.
    // `before` optionally defines the element we'll use as a reference for injecting our <link>
    // By default, `before` uses the first <script> element in the page.
    // However, since the order in which stylesheets are referenced matters, you might need a more specific location in your document.
    // If so, pass a different reference element to the `before` argument and it'll insert before that instead
    // note: `insertBefore` is used instead of `appendChild`, for safety re: http://www.paulirish.com/2011/surefire-dom-element-insertion/
    var ss = window.document.createElement( "link" );
    var ref = before || window.document.getElementsByTagName( "script" )[ 0 ];
    var ssLength = window.document.styleSheets.length;
    ss.rel = "stylesheet";
    ss.href = href;
    // temporarily, set media to something non-matching to ensure it'll fetch without blocking render
    ss.media = "only x";
    // inject link
    ref.parentNode.insertBefore( ss, ref );
    // This function sets the link's media back to `all` so that the stylesheet applies once it loads
    // It is designed to loop until document.styleSheets includes
    function toggleMedia(){
        if( window.document.styleSheets.length > ssLength ){
            ss.media = media || "all";
        }
        else {
            setTimeout( toggleMedia );
        }
    }

    toggleMedia();
    return ss;
 }

This works generally, but a downside to it is it may not work well with multiple loadCSS calls, since they'll all toggle their media as soon as the first call goes out. I think we might be able to work around this by queueing the media toggles in the order they're called. Maybe a loadCSS.locked property would give us that sort of mechanism. The queue wouldn't incur much delay, since it's not about waiting for requests to return so much as ensuring the stylesheets have joined the CSSOM.

Since it's high-priority, we'll try and close this one out in the morning.

@scottjehl scottjehl self-assigned this Oct 23, 2014
@adammcarth
Copy link
Contributor

Seems to work alright on 4G ;)

@decrek
Copy link

decrek commented Oct 24, 2014

I suggested earlier in the mail I send you yesterday the xhr method. I understand that we come across browser support complexities. So I understand the polling approach as well. I started thinking and came across http://stackoverflow.com/q/2635814.
So how about polling ss.sheet? Once it is not null anymore the stylesheet is loaded and it should be safe to toggle the media back. Tested it and worked on Chrome. With 2 css calls, one for main.css and one for the font.css. Something like below:

function loadCSS( href, before, media ){
    "use strict";
    // Arguments explained:
    // `href` is the URL for your CSS file.
    // `before` optionally defines the element we'll use as a reference for injecting our <link>
    // By default, `before` uses the first <script> element in the page.
    // However, since the order in which stylesheets are referenced matters, you might need a more specific location in your document.
    // If so, pass a different reference element to the `before` argument and it'll insert before that instead
    // note: `insertBefore` is used instead of `appendChild`, for safety re: http://www.paulirish.com/2011/surefire-dom-element-insertion/
    var ss = window.document.createElement( "link" );
    var ref = before || window.document.getElementsByTagName( "script" )[ 0 ];
    var ssLength = window.document.styleSheets.length;
    ss.rel = "stylesheet";
    ss.href = href;
    // temporarily, set media to something non-matching to ensure it'll fetch without blocking render
    ss.media = "only x";
    // inject link
    ref.parentNode.insertBefore( ss, ref );
    // This function sets the link's media back to `all` so that the stylesheet applies once it loads
    // It is designed to loop until document.styleSheets includes
    function toggleMedia(){
        if(ss.sheet ){
            ss.media = media || "all";
        }
        else {
            setTimeout( toggleMedia );
        }
    }

    toggleMedia();
    return ss;
}

In the link I pasted I see some if else, probably because ss.sheet is not supported everywhere. What do you think?

@scottjehl
Copy link
Member Author

Thanks @decrek. I like that, though I'd prefer something that came earlier than when the CSS finishes loading, since that'd be a great deal of polling. Regardless, I like the idea and will be poking at it a bit today.

scottjehl added a commit that referenced this issue Oct 24, 2014
… in the CSSOM, and essentially loaded. This ensures that the sheet loads with a low-priority media query and the real media is set at a safe time. Fixes #19
@scottjehl
Copy link
Member Author

Hi all.
I've posted a proposed approach to the issue-19 branch and I wouldn't mind some feedback on it. It basically polls document.styleSheets until the sheet we want is defined in there by its href property. It has a similar result to @decrek's proposal above, but hopefully without the workarounds that the sheet property would have required (I'm testing this now).

29ea3d7

@decrek
Copy link

decrek commented Oct 24, 2014

I tested this again with my insanely throttled connection and works(tested in Chrome) Btw I use Charles for that. Seriously I never been so deep into that cssom thing, so really don't know how that sheet property is supported. So I am curious about your testing results. AFAIK the href property is supported everywhere. I also see that in my proposal I poll until the stylesheet is loaded like you said in your reply and in your latest proposal you poll until it is present in the DOM (and started downloading?). That would be better.
Just for my understanding, the CSS request will not be blocking when it's started and has a non matching media property, right? That would mean that the moment the request is started you can toggle the media property back?

@scottjehl
Copy link
Member Author

@decrek yes, that's the case (a non-applicable media type will be downloaded at a lower priority, async, in modern browsers). It seems that the href property is populated when a stylesheet finishes loading, rather than when it is first requested, which is unfortunate since it will mean more polling. The only approach that I've found so far that allows us to detect the moment the request is made is to check stylesheets.length for an increment, but then we run into issues with multiple loadCSS calls.

This approach seems to work most broadly of those I've tested so far.

@decrek
Copy link

decrek commented Oct 24, 2014

@scottjehl Ah ok, that means the same amount of polling as in my proposal but better support than the sheet property.

@scottjehl
Copy link
Member Author

yeah, that’s what I’m thinking at least.

On Oct 24, 2014, at 1:07 PM, Declan Rek notifications@github.com wrote:

@scottjehl Ah ok, that means the same amount of polling as in my proposal but better support than the sheet property.


Reply to this email directly or view it on GitHub.

@scottjehl
Copy link
Member Author

This is merged now. @addyosmani I'd be interested in your thoughts on any better approach now that we have something workable at least.

@zachleat
Copy link
Member

For posterity, the document.styleSheets addition after load is a WebKit/Blink behavior. In Gecko the document.styleSheets addition happens immediately after the stylesheet node is appended to the document.

Probably worth testing on a Firefox phone, if anyone has one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants