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

Remove core/ready hard dependency on $.Deferred #1778

Closed
mgol opened this issue Oct 21, 2014 · 16 comments
Closed

Remove core/ready hard dependency on $.Deferred #1778

mgol opened this issue Oct 21, 2014 · 16 comments

Comments

@mgol
Copy link
Member

mgol commented Oct 21, 2014

Originally reported by m_gol at: http://bugs.jquery.com/ticket/15174

If $.Deferred is unavailable (e.g. because it was removed via custom compilation), the core/ready implementation should fall back to the standard Promise.

Issue reported for jQuery 2.1.1

@mgol mgol self-assigned this Oct 21, 2014
@mgol mgol added this to the 3.0.0 milestone Oct 21, 2014
@mgol mgol added the Feature label Oct 21, 2014
@dmethvin
Copy link
Member

Also related to gh-1823

@timmywil
Copy link
Member

Hmm, I don't think this is necessary. Deferred is simply a hard dependency of core/ready. If Deferred is removed from a custom build, so core/ready should be too (which we could easily add to the Gruntfile). Those making custom builds are usually also accustomed to placing scripts at the end of the body, making core/ready wasted space. Maybe this is a docs issue instead.

@scottgonzalez
Copy link
Member

.ready() is a pretty important API. I don't think it should go away just because someone doesn't want $.Deferred. Also, I thought there were still some cases where just putting scripts at the end of the body isn't enough. I think @gibson042 might remember when that's the case?

@timmywil
Copy link
Member

I used to think that too. We had this conversation a while ago and it turned out it is sufficient.

Also, Deferred is an important API too. It is used internally in other areas besides core/ready. core/ready is probably the least important of them all.

@timmywil
Copy link
Member

I did think of a limitation. On a very large document, you'd probably prefer to start downloading the script immediately, but still defer so the browser can download and parse the DOM in parallel. In this case, doc ready can still play a role just in case the script loads before the document is ready, but I think script loaders often have that covered.

@timmywil
Copy link
Member

Ah, here is the conversation I was thinking of. That only overlaps because I don't remember finding evidence that putting scripts at the end if the body wouldn't work.

@timmywil
Copy link
Member

You know what, I think I've already gone over to the other side. Sorry for the brain dump of notifications. This could be useful to users. My only question is, how do we handle the lack of native Promise in certain environments?

@mgol
Copy link
Member Author

mgol commented Jan 31, 2015

Up until very recently we used to divide support tests into two parts:
one that didn't require attaching anything to document that was invoked at
start and one that did that was invoked on doc ready. Therefore, if you use
any jQuery earlier than 1.11/2.1 (a.k.a. latest), you have to run your code
on doc ready or you risk stumbling upon bugs as the one linked to by
timmywil in this thread.

Now we have this second part of support tests lazy; in 2.x they have
elements attached to document.documentElement so this can happen anytime;
in 1.x they are attached to document.body and you'll have no results until
document.body exists; you'll be fine if you put scripts at the end of body.

What bothers me most here is that if you do depend on core/ready and
consume it via AMD, you'll get Deferred pulled into automatically by
RequireJS and - later - the builder. Of course if we determined that
relying on core/ready is no longer necessary then the importance of this
fact decreases.

I was thinking that if you exclude Defered and not core/ready you're
required to provide window.Promise. Now, that approach will blow up for
people using our AMD modules that will not use $.ajax, .animate or Deferred
by themselves but will use core/ready...

Michał Gołębiowski

@dmethvin
Copy link
Member

I think it's reasonable to focus on doing any dependency reduction just on master and not compat.

How about this? When Deferred is missing we just do something simple like convert .ready(handler) to window.addEventListener("DOMContentLoaded", function(){ handler(jQuery); }). We wouldn't need to maintain the queue ourselves, the browser should maintain FIFO order, and handlers that throw exceptions don't affect subsequent ones.

At the moment, jQuery.ready is undocumented. We've stated the intention to document it in jquery/api.jquery.com#205 but as a thenable and not specifically as a Deferred or Promise. For consistency of behavior in all the edge cases I think it's best to use Deferred if it's there. Our docs should tell anyone doing a custom build without Deferred that they need to provide a Promise shim.

@scottgonzalez
Copy link
Member

Wouldn't that break the usage of .ready() after the event has occurred?

@dmethvin
Copy link
Member

In the current ready module there's already plumbing to handle that, although it might be complicated by holdReady().

@gibson042
Copy link
Member

I'd like to take a dependency-free Promise-compatible approach (Promise.resolve( $.ready ), $.when( $.ready ), etc.)—no shims, no gaps, no build restrictions:

var readyFiring = 0,
    readyCallbacks = [],
    whenReady = function( callback ) {
        readyCallbacks.push( callback );
    };

jQuery.fn.ready = function( callback ) {
    whenReady( callback );
    return this;
};

// This could also be a thenable function if we want jQuery.ready( fn )
// ...but I don't
jQuery.ready = { then: jQuery.fn.ready };

// Suitably wired to load events, document.readyState, and holdReady
function resolveReady() {
    whenReady = function( callback ) {
        readyCallbacks.push( callback );
        if ( !readyFiring++ ) {
            while ( readyCallbacks.length ) {
                callback = readyCallbacks.shift();
                if ( jQuery.isFunction( callback ) ) {
                    // Perfect backcompat: synchronous call with context and no try/catch
                    // ...but we can break a bit with 3.0 if we want
                    callback.call( document, jQuery );
                }
            }
        }
        readyFiring--;
    };
    whenReady();
}

And additionally:

  • Deprecate jQuery( document ).ready( fn ) etc. in favor of jQuery( fn )
  • Deprecate jQuery.holdReady (a nasty global behavior modifier better covered by case-specific deferred/promise patterns)
  • Remove the already-deprecated document "ready" event

@scottgonzalez
Copy link
Member

All of the additionally's sound good to me.

@dmethvin
Copy link
Member

dmethvin commented Feb 5, 2015

Oh that's a really simple way to patch things, and I like the fact it doesn't have dependencies. I agree with the extras as well. I never really liked jQuery.holdReady and it doesn't seem to be widely used, seems like maybe an older stealJS uses it? And in any case we're not talking about removal at this point, just deprecation.

@Krinkle
Copy link
Member

Krinkle commented Feb 28, 2015

Note that, for better or worse, MediaWiki currently makes use of jQuery.ready() (the method, not the thenable). View source of any Wikipedia page for example.

It uses it to fire off jQuery's dom-ready handlers towards the end of <body>, just before the actual end (there's a few script tags after it).

</div>
<script>window.jQuery && jQuery.ready();</script>
<script>if(window.mw){
mw.loader.require( .. );
}</script>
<script src="/static/src/.."></script>
<script>if(window.mw){
mw.data.set({backendResponseTime:24,backendHost:"mw1240"});
}</script>
</body>
</html>

It solved a Firefox bug with regards to document.write.

It also simplified things a little by not needing jQuery to do all its cross-browser stuff.

And it served as a way to effectively bring <script async> to all browsers (in that, the bootstrapping scripts loaded from <head> that run at document ready, can now run before the scripts loaded from the bottom of body are requested/loaded/parsed, thus making it a lot more likely that they run before the initial rendering and reduce flashes of lesser styled content).

@mgol
Copy link
Member Author

mgol commented Jul 27, 2015

@gibson042 Seems by #1778 (comment) that you have a good idea of how to do it; would you like to get this issue re-assigned to you?

@timmywil timmywil assigned gibson042 and unassigned mgol Jan 15, 2016
timmywil added a commit to timmywil/jquery that referenced this issue Jan 19, 2016
- Keeps is Promise-compatible

Fixes jquerygh-1778
timmywil added a commit to timmywil/jquery that referenced this issue Jan 19, 2016
- Keeps it Promise-compatible

Fixes jquerygh-1778
@timmywil timmywil self-assigned this Jan 19, 2016
timmywil added a commit to timmywil/jquery that referenced this issue Feb 1, 2016
timmywil added a commit to timmywil/jquery that referenced this issue Feb 1, 2016
timmywil added a commit to timmywil/jquery that referenced this issue Mar 28, 2016
- Keeps it Promise-compatible

Fixes jquerygh-1778
timmywil added a commit to timmywil/jquery that referenced this issue Mar 28, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

7 participants