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

<script defer> fires after $(function) in Chrome and FX #3271

Open
KeithHenry opened this issue Aug 10, 2016 · 16 comments

Comments

Projects
None yet
6 participants
@KeithHenry
Copy link

commented Aug 10, 2016

In Chrome, FX and IE >=10 <script defer> should cause scripts to be background loaded, but then executed in order before DOMContentLoaded fires.

However, when lots of scripts are loaded with defer applied the jQuery event fires before all the scripts have run.

You can test this with:

file 1.js:

window.test = "fail init"; 
// test result when jQuery ready fires
$(function() {$('#resultJQ').html("jQuery result: " + window.test);});

// test result when document is ready
document.addEventListener("DOMContentLoaded", function() {$('#resultDOM').html("DOM result: " + window.test);});

file 2-9.js:

window.test = "fail " + n; 

file 10.js:

window.test = "pass" 

page.html:

<!DOCTYPE html>
<html>
<head>
    <script type="text/javascript" src="jquery.js" defer></script>
    <script type="text/javascript" src="1.js" defer></script>
    ...
    <script type="text/javascript" src="n.js" defer></script>
    ...
    <script type="text/javascript" src="10.js" defer></script>
</head>
<body>
    <div id="resultJQ">waiting...</div>
    <div id="resultDOM">waiting...</div>
<body>
</html>

Both tests pass when files load fast enough, but sufficient delays (>50ms each) in the *.js files cause the jQuery test to fail in latest Chrome and FX. It doesn't fail consistently, but the jQuery ready always fires before the document one.

Here is a jsBin that reproduces it on first load, although this might depend on your network and subsequent loads won't show the bug. Try throttling your connection and CTRL+F5 if it doesn't happen on first load.

This bug is extant in the latest 2.x and 3.x but doesn't seem to occur in 1.x

This appears to be due to this check:

// Catch cases where $(document).ready() is called
// after the browser event has already occurred.
// Support: IE <=9 - 10 only
// Older IE sometimes signals "interactive" too soon
if ( document.readyState === "complete" ||
    ( document.readyState !== "loading" && !document.documentElement.doScroll ) ) {

    // Handle it asynchronously to allow scripts the opportunity to delay ready
    window.setTimeout( jQuery.ready );
}

When debugging it appears that document.readyState === 'interactive' and !document.documentElement.doScroll is true, so jQuery.ready fires with the minimum delay (4ms). Breaking on window.setTimeout halts execution while some of the numbered js files are still pending in the Network tab.

I don't know whether this is a recent change in behavior, but Chrome and Firefox appear to report document.readyState === 'interactive' as soon as the body is present, but before deferred scripts are evaluated.

This isn't a problem in IE < 10 as it doesn't support defer (it sort of does, but fires scripts out of order and so would fail this test anyway).

@KeithHenry KeithHenry changed the title <script defer> fires before $(function) in Chrome and FX <script defer> fires after $(function) in Chrome and FX Aug 10, 2016

@dmethvin

This comment has been minimized.

Copy link
Member

commented Aug 10, 2016

What a refreshing change! Our recent complaints have been about .ready() firing too late, and here's one about it firing too soon. 😄

There's a long history with us trying to use "interactive" and it looks like we may just need to revert it and put a comment there that says // NO REALLY YOU CAN'T. See dabd5ba, gh-2100, and trac-10067.

@KeithHenry

This comment has been minimized.

Copy link
Author

commented Aug 10, 2016

I think having events fire on interactive makes sense, especially if you're working to reduce the time to first meaningful interaction, but it can't be the same as ready - there are two events:

  • Do something as soon as the user can interact with the page.
  • Do something once all the javascript libraries that need to be loaded have been.

Both are needed, but they're not the same thing.

@dmethvin

This comment has been minimized.

Copy link
Member

commented Aug 11, 2016

@KeithHenry I agree both can be useful, but $(fn) has to have its semantics defined. We either need to ensure fn doesn't run before defer scripts load/execute or say that we don't support loading jQuery itself using the defer attribute.

@KeithHenry

This comment has been minimized.

Copy link
Author

commented Aug 11, 2016

Exactly.

I'd argue that $(document).ready(function) should mean fully ready, and $(document).interactive(function) would be new functionality.

As long as it's clear and explicit what event jQuery is firing on then that should resolve this bug.

@timmywil

This comment has been minimized.

Copy link
Member

commented Sep 26, 2016

The problem is there doesn't seem to be a way to get in before the event loop starts, but after scripts have loaded and executed. You either have to wait until the ready state is complete, which is potentially many event loops after DOMContentLoaded or stick with interactive, which seems to be the right move in most cases. In other words, dropping interactive makes ready just like load, and the only advantage is that jQuery can be loaded with the defer attribute. If all the scripts were moved to the end of the body, you'd get the same advantage of parsing all the DOM before parsing/executing scripts without the need to drop interactive.

I wish there was a web API to actually check DOMContentLoaded, but the ideal would be something that behaves like this.

<script>
document.addEventListener('DOMContentLoaded', function() {
  window.DOMLoaded = true;
});
</script>
<script type="text/javascript" src="jquery.js" defer></script>
<script type="text/javascript" src="1.js" defer></script>
...
<script type="text/javascript" src="10.js" defer></script>

Then we (or the user) could check window.DOMLoaded. Neither complete nor interactive really cuts it.

All that to say, a perfect .ready is impossible. This is a choice between interactive or complete. It pains me to say it, but I think we have to drop interactive. It would be worse to document that jQuery can't be loaded with the defer attribute.

@timmywil timmywil added this to the 3.2.0 milestone Sep 26, 2016

@timmywil timmywil self-assigned this Sep 26, 2016

@timmywil timmywil added the Core label Sep 26, 2016

@neoncube2

This comment has been minimized.

Copy link

commented Dec 3, 2016

Do you think something like this would work?

if ( document.readyState === "complete") {
	window.setTimeout( jQuery.ready );
}
else if( document.readyState === "interactive" ) {
    document.onreadystatechange = function () {
      if ( document.readyState == "complete" ) {
          window.setTimeout( jQuery.ready );
      }
  }
}
else {
    document.addEventListener( "DOMContentLoaded", completed );
  
    window.addEventListener( "load", completed );
}

The idea is that if ready() is called when readyState is completed, then the ready handler would fire pretty much immediately, but if readyState was interactive, then it would wait until readyState was completed to fire. If readyState was neither completed or interactive, then the DOMContentLoaded event would be used.

I think this should preserve the desired behavior except for in some cases where defer is used, in which case ready() would be the same as load(), which seems like it might be okay.

@mgol

This comment has been minimized.

Copy link
Member

commented Mar 7, 2017

@timmywil If a conclusion is to revert the workaround perhaps we could land it in 3.2.1 instead of waiting for 3.3.0? The patch will be simple, the main problem is testing that thing (but an iframe test should do it, I guess).

@gkatsanos

This comment has been minimized.

Copy link

commented May 22, 2017

TL/DR is "don't load jQuery with defer" ? I struggled hours with a bug before I ended up here :/ (and yes removing defer solves it)

@neoncube2

This comment has been minimized.

Copy link

commented May 30, 2017

@gkatsanos Yes, I believe that's the current state of things.

@neoncube2

This comment has been minimized.

Copy link

commented Aug 6, 2017

I've done some testing, and it appears that replacing the code that tests for ready state with the following code fixes the issue for me, at least on a Chromium browser.

// Catch cases where $(document).ready() is called
// after the browser event has already occurred.
if ( document.readyState === "complete" ) {

	// Handle it asynchronously to allow scripts the opportunity to delay ready
	window.setTimeout( jQuery.ready );
} else if ( document.readyState === "interactive" ) {
    document.onreadystatechange = function () {
      if ( document.readyState == "complete" ) {
          window.setTimeout( jQuery.ready );
      }
  }
} else {

	// Use the handy event callback
	document.addEventListener( "DOMContentLoaded", completed );

	// A fallback to window.onload, that will always work
	window.addEventListener( "load", completed );
}

If the document is in the "interactive" state, then the code waits for the document to be in the "complete" state and then runs the ready handler. This means that when jQuery is loaded with "defer", the ready handler is fired only when the page has completely finished loading and any other deferred scripts have run, but when jQuery is not loaded with defer, it tries to use DOMContentLoaded() or immediately fires the ready handler.

The idea is that DOMContentLoaded is still used except for when jQuery is loaded with "defer", in which case it waits for the whole document to be completely loaded before the ready handler is fired. That isn't ideal, but I think it's better than always waiting for the document to be completely loaded before the ready handler is run.

Hopefully this is helpful and I'm not missing something. If anyone could test this out, that would be great.

@timmywil

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

@neoncube2 That sounds like a reasonable compromise.

@KeithHenry

This comment has been minimized.

Copy link
Author

commented Aug 7, 2017

@neoncube2 Won't that just break it for the users that expect ready to fire early? I don't think the issue here is that it's hard to fix, I think it's that an unknown number of existing jQuery components out there expect ready to fire early and would break if that behaviour changes to fire when the page is actually ready.

Fixing this would potentially reopen #2100

@timmywil

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

@KeithHenry Yea, I just re-read the code. Looks like that's just waiting for load. I was thinking there could be a compromise that wouldn't change behavior for those not using defer.

@neoncube2

This comment has been minimized.

Copy link

commented Aug 8, 2017

@KeithHenry and @timmywil

Ah, I think you may be right.

Is what I've outlined below the behaviour that we're going for, then?

For when jQuery is loaded with defer:

  • If readyState is interactive, run the ready handler, but only after all other scripts that were loaded with defer have run.
  • If readystate is complete, immediately run the ready handler
  • Otherwise, run the ready handler on DOMContentLoaded()

For when jQuery isn't loaded with defer:

  • If readyState is interactive or complete, run the event handler immediately.
  • Otherwise, run the ready handler on DOMContentLoaded()

Is that right?

Also, @timmywil mentioned that if we could tell if DOMContentLoaded() had already been fired, that might be helpful, and I think that might be possible using PerformanceTiming.domContentLoadedEventStart, although that might not be the direction you want to go.

@KeithHenry

This comment has been minimized.

Copy link
Author

commented Aug 8, 2017

@neoncube2 not really - jQuery is already good at firing ready if called after the DOM events - that isn't the issue.

The problem is that that ready fires as soon as the page is interactive, and an unknown number of projects that depend on jQuery rely on that behaviour.

To work when defer is set jQuery needs to delay ready until document.readyState === 'complete', and never fire when document.readyState === 'interactive' but doing so will be a breaking change for these unknown projects. There may even be projects out there that use defer and rely on the current broken behaviour.

I suppose you could pass some kind of defer-mode to jQuery in order to change the behaviour, but then you're either adding to an already fairly large library or splitting the code to deliver an alternate variant, and adding complexity and technical debt either way.

@mgol

This comment has been minimized.

Copy link
Member

commented May 15, 2019

@alex-jitbit Thanks for the info. Is the large number of deferred scripts needed to hit that issue or is a single bug long-loading one enough to hit it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.