-
Notifications
You must be signed in to change notification settings - Fork 264
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
Conversation
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><body onload=""></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> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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."
Hi @mattkime. Did you have the time to update your PR? I'd like to merge it. |
Friendly ping @mattkime. |
Hey @mattkime, do you have any updates for us? Thanks :) |
apologies for the delays, looking at this. |
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 |
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?
|
This is a great news @dmethvin! |
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! |
No description provided.