Skip to content

warning for using .ready() when async loading jQuery #751

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

Closed
wants to merge 3 commits into from

Conversation

mattkime
Copy link

@mattkime mattkime commented Jun 5, 2015

No description provided.

@mattkime
Copy link
Author

anything i can do to move this forward? i spent a bunch of time learning this on my own. would have been much easier if it had been documented.

@@ -14,6 +14,7 @@
<div class="warning">
<p>The <code>.ready()</code> method is generally incompatible with the <code>&lt;body onload=""&gt;</code> attribute. If <code>load</code> must be used, either do not use <code>.ready()</code> or use jQuery's <code>.load()</code> method to attach <code>load</code> event handlers to the window or to more specific items, like images.
</p>
<p>Loading jQuery asynchronously may cause jQuery to load after <code>DOMContentReady</code>, resulting in <code>.ready()</code> firing on <code>load</code>. Due to browser quirks jQuery is unable to determine whether <code>DOMContentLoaded</code> has fired and falls back to <code>load</code>. This can be fixed two ways. Loading jQuery synchronously will fix it. You can also place your async script requests near the closing of the body tag and remove <code>.ready()</code> usage. You code will be loaded after <code>DOMContentReady</code> fires and can execute immediately. </p>
Copy link
Member

Choose a reason for hiding this comment

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

Here:

Due to browser quirks jQuery

I'd place a comma before jQuery

Here:

This can be fixed two ways. Loading jQuery

I'd have:

This can be fixed in two ways. The first one is loading jQuery

Here:

immediately.

there's a trailing white space.

Copy link
Member

Choose a reason for hiding this comment

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

The last few sentences sound awkward to me. I think the paragraph would read better if it stuck to async requests. How does something like this sound?

Loading jQuery asynchronously may cause jQuery to load after DOMContentReady, resulting in .ready() firing on load. Due to browser quirks, jQuery is unable to determine whether DOMContentLoaded has fired and falls back to load. To avoid this problem with asynchronous script requests, place those requests near the closing of the body and remove .ready() usage.

Copy link
Author

Choose a reason for hiding this comment

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

I think the bit about placing async script requests needs to be removed. I'm about 80% certain that it always works which is about 20% short of good advice. I was also doing some perf testing and found that this resulted in much slower script loading - Chrome would end up downloading scripts after images. Browsers are still browsers.

RequireJS provides https://github.com/requirejs/domReady - perhaps we should "Check your script loader docs for a $.ready alternative."

@AurelioDeRosa
Copy link
Member

Hi @mattkime. Did you have the time to update your PR? I'd like to merge it.

@AurelioDeRosa
Copy link
Member

Friendly ping @mattkime.

@arthurvr
Copy link
Member

Hey @mattkime, do you have any updates for us? Thanks :)

@mattkime
Copy link
Author

apologies for the delays, looking at this.

@mattkime
Copy link
Author

fwiw - my team had some back and forth over removing .ready() calls. it was an unpopular option. ultimately we found sync script loading to be faster. ymmv.

here's another take on the text. i'm not too picky about the details but want to make sure its good, succinct advice.

Loading jQuery asynchronously may cause jQuery to load after DOMContentReady, resulting in .ready() firing on load. Due to browser quirks, jQuery is unable to determine whether DOMContentLoaded has fired and falls back to load. Either load jQuery synchronously or consult your script loader docs.

@dmethvin
Copy link
Member

Just another complication here, in 3.0 we try to run ready scripts earlier so they don't always wait until the page is fully loaded. So the wording needs to clarify that as well. How about this?

Prior to version 3.0, when a script is loaded asynchronously after the DOMContentLoaded event has already fired, functions added by calls to the .ready() method do not run until the entire document has loaded (essentially, window.onload). As of jQuery 3.0, these functions run immediately.

@AurelioDeRosa
Copy link
Member

This is a great news @dmethvin!

@dmethvin
Copy link
Member

This PR has gone stale, but there is discussion going on in jquery/jquery#3271 that will impact behavior, docs, or both. Thanks for bringing this to our attention @mattkime!

@dmethvin dmethvin closed this Aug 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants