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

Document jQuery.ready as Promise-consumable #530

Closed
wants to merge 1 commit into from
Closed

Document jQuery.ready as Promise-consumable #530

wants to merge 1 commit into from

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Jul 15, 2014

Also:

  • Categorise jQuery.holdReady in events/document-loading,
    to match ready() and jQuery.ready.promise().
  • Categorise jQuery.holdReady in properties/global-jquery-object-properties,
    to match jQuery.fx.off() and jQuery.ready.promise().

Fixes #205

</argument>
</signature>
<longdesc>
<p>The <code>$.ready.promise()</code> method provides a Promise interface to the document ready event. See also <code><a href="/ready/">ready()</a></code>, which makes use of this.</p>
Copy link
Member

Choose a reason for hiding this comment

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

In the standards sense, it doesn't provide a Promise interface but instead a thenable one. This has been one of the reasons we've been hesitant to document this. The other is that we may try to make jQuery.Deferred optional in a future version, so it's almost as if we're documenting and deprecating this at the same time.

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand it isn't a standards Promise (the definition of that seems still in flux, too), I just tried to match our terminology from elsewhere. What should this be called?

  • deferred.promise: "type=Promise", "Return a Deferred's Promise object.", "The Promise exposes only the Deferred methods needed to attach additional handlers or determine the state"
  • Types: "Promise Object", "a subset of the methods of the Deferred object"
  • promise: "a Promise object to observe when all [actions] have finished"

@dmethvin
Copy link
Member

The holdReady changes LGTM but the ones for jQuery.ready.promise need some more discussion. We currently don't even document jQuery.ready (only jQuery().ready) so that alone is bothersome.

@Krinkle
Copy link
Member Author

Krinkle commented Jul 15, 2014

The jQuery.ready function seems an internal method though. Perhaps this .promise should be moved to somewhere else? Or we could rename this doc file to jQuery.ready and document it as an object with a .promise() method.

Also:
* Categorise jQuery.holdReady in events/document-loading,
  to match ready() and jQuery.ready.promise().
* Categorise jQuery.holdReady in properties/global-jquery-object-properties,
  to match jQuery.fx.off() and jQuery.ready.promise().

Fixes #205
@dmethvin
Copy link
Member

Agreed, jQuery.ready is an internal method so putting a public property on it seems like a bad idea. We kind of did it with jQuery.fx.off before but no reason to have bad follow bad. And if we move the location then the whole idea is up for grabs again. Adding an implied dependency on either $.Deferred or a standards-based Promise seems like a bad idea at this point when we're trying to encourage modularity. It's pretty trivial to create your own thenable of any flavor at the application level, right?

@gnarf
Copy link
Member

gnarf commented Dec 21, 2014

I actually think it might be as easy as var docReady = new Promise(jQuery)

@gnarf
Copy link
Member

gnarf commented Dec 22, 2014

@Krinkle I don't think we want to document the .ready.promise but I do think we want to add the categories to the .holdReady - Going to bring this pull up in the core meeting today to ask opinion from the team.

@jaubourg
Copy link
Member

$.when(
    $.ajax( templateURL ),
    $.ajax( dataURL ),
    $.ready
).then( function( template, data ) {
   $( "#target" ).html( applyTemplate( template[ 0 ], data[ 0 ] ) );
} );

This is how you decouple initialization code while enabling preemptive asynchronous operations and keeping everything readable. Having a promise method on jQuery.ready makes it a breeze and, yes, it should be documented.

@dmethvin
Copy link
Member

Yes, the ready promise is nice to use. Two issues with it:

  • It is in a bad place. jQuery.ready is not a public property or method. If we want a documented ready promise we can put it elsewhere and the jQuery Migrate plugin can warn about the old undocumented use.
  • It needs to be really clearly documented this property has a dependency on jQuery.Deferred and excluding that module means the promise won't be created. Unless we do something to use other promise implementations, in which case that also needs to be documented.

Edit by @gnarf - made into checkboxes

@gnarf
Copy link
Member

gnarf commented Dec 22, 2014

Okay, $.when( $.ready ) is super useful, but I think we should make a few changes, do a few checks first here:

  • Don't document calling .promise() directly in examples, just use it with $.when( $.ready ).done everywhere
  • Ensure the behavior is in a unit test suite somewhere, otherwise, much like my favorite $({}).queue() is probably just an abomination known only to people who have watched our conference talks or read our stack overflow answers...

@gnarf
Copy link
Member

gnarf commented Dec 22, 2014

@arthurvr arthurvr mentioned this pull request Jan 2, 2015
@AurelioDeRosa
Copy link
Member

Is this PR still relevant? It's more than a year old.

@agcolom
Copy link
Member

agcolom commented Jan 27, 2016

@timmywil Could you please check this? Thanks (old PR).

@timmywil
Copy link
Member

Oh, sorry. It depends on the resolution of the ticket @gnarf linked. I guess it's been longer than I thought, but we finally have a PR for it. I'm on the fence about whether it's worth the bytes. If we merge the PR, jQuery.ready.promise will no longer exist. Instead, we'll probably want to document that jQuery.ready is "Promise-consumable [opposite of sic]".

@gnarf
Copy link
Member

gnarf commented Feb 3, 2016

Thenable?
On Jan 28, 2016 09:31, "Timmy Willison" notifications@github.com wrote:

Oh, sorry. It depends on the resolution of the ticket @gnarf
https://github.com/gnarf linked. I guess it's been longer than I
thought, but we finally have a PR
jquery/jquery#2850 for it. I'm on the fence
about whether it's worth the bytes. If we merge the PR,
jQuery.ready.promise will no longer exist. Instead, we'll probably want
to document that jQuery.ready is "Promise-consumable [opposite of sic]".


Reply to this email directly or view it on GitHub
#530 (comment)
.

@timmywil
Copy link
Member

timmywil commented Feb 3, 2016

It looks like we will be getting rid of jQuery.ready.promise. I can say with some certainty that we will instead want to document jQuery.ready as Promise-consumable. As @gnarf said, "thenable" is also appropriate. Example usage would be Promise.resolve(jQuery.ready) with native Promise.

@timmywil timmywil changed the title Document jQuery.ready.promise() Document jQuery.ready as Promise-consumable Feb 3, 2016
@tdelmas
Copy link

tdelmas commented Jun 9, 2016

Shouldn't that pull request be merged? https://jquery.com/upgrade-guide/3.0/ says that "Feature: jQuery.ready promise is formally supported" but I don't see it on https://api.jquery.com

@timmywil
Copy link
Member

timmywil commented Jun 9, 2016

The PR needs updating first.

@mgol
Copy link
Member

mgol commented Jul 13, 2016

jquery/jquery#1778 has been fixed so this PR should no longer be blocked.

@mgol mgol added this to the 3.0.0 milestone Jul 13, 2016
@dmethvin
Copy link
Member

@Krinkle With jQuery 3.0 we're exposing jQuery.ready only as a thenable, so the example here that says jQuery.ready.done is not supported. There is a non-Deferred alternate module and in that build the code would break. Instead, we're [advising}(https://jquery.com/upgrade-guide/3.0/#feature-jquery-ready-promise-is-formally-supported) that people always use jQuery.when( jQuery.ready ) to convert to a Deferred or Promise.resolve( jQuery.ready ) if someone wanted a native Promise.

Would you like to update this PR to reflect those changes? It would be great to get that done!

@dmethvin dmethvin mentioned this pull request Aug 12, 2016
21 tasks
@Krinkle Krinkle mentioned this pull request Sep 20, 2016
@Krinkle
Copy link
Member Author

Krinkle commented Sep 20, 2016

@dmethvin Closing this pull request because I deleted and re-forked this repo last year, and there doesn't appear to be anyway to restore the head of a pull request (π). Re-created as #983.

@Krinkle Krinkle closed this Sep 20, 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.

$.ready documentation
9 participants