Document jQuery.ready as Promise-consumable #530

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
9 participants
@Krinkle
Member

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

@dmethvin

View changes

entries/jQuery.ready.promise.xml
+ </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>

This comment has been minimized.

@dmethvin

dmethvin Jul 15, 2014

Member

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.

@dmethvin

dmethvin Jul 15, 2014

Member

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.

This comment has been minimized.

@Krinkle

Krinkle Jul 15, 2014

Member

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"
@Krinkle

Krinkle Jul 15, 2014

Member

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

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jul 15, 2014

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.

Member

dmethvin commented Jul 15, 2014

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

This comment has been minimized.

Show comment
Hide comment
@Krinkle

Krinkle Jul 15, 2014

Member

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.

Member

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.

Document jQuery.ready.promise()
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

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jul 16, 2014

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?

Member

dmethvin commented Jul 16, 2014

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

This comment has been minimized.

Show comment
Hide comment
@gnarf

gnarf Dec 21, 2014

Member

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

Member

gnarf commented Dec 21, 2014

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

@gnarf

This comment has been minimized.

Show comment
Hide comment
@gnarf

gnarf Dec 22, 2014

Member

@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.

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

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Dec 22, 2014

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.

Member

jaubourg commented Dec 22, 2014

$.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

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 22, 2014

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

Member

dmethvin commented Dec 22, 2014

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

This comment has been minimized.

Show comment
Hide comment
@gnarf

gnarf Dec 22, 2014

Member

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...
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

This comment has been minimized.

Show comment
Hide comment
@gnarf

gnarf Dec 22, 2014

Member
Member

gnarf commented Dec 22, 2014

@arthurvr arthurvr referenced this pull request Jan 2, 2015

Closed

$.ready documentation #205

@AurelioDeRosa

This comment has been minimized.

Show comment
Hide comment
@AurelioDeRosa

AurelioDeRosa Nov 15, 2015

Member

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

Member

AurelioDeRosa commented Nov 15, 2015

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

@agcolom

This comment has been minimized.

Show comment
Hide comment
@agcolom

agcolom Jan 27, 2016

Member

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

Member

agcolom commented Jan 27, 2016

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

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jan 28, 2016

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]".

Member

timmywil commented Jan 28, 2016

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

This comment has been minimized.

Show comment
Hide comment
@gnarf

gnarf Feb 3, 2016

Member

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)
.

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

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Feb 3, 2016

Member

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.

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 from Document jQuery.ready.promise() to Document jQuery.ready as Promise-consumable Feb 3, 2016

@tdelmas

This comment has been minimized.

Show comment
Hide comment
@tdelmas

tdelmas Jun 9, 2016

Contributor

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

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jun 9, 2016

Member

The PR needs updating first.

Member

timmywil commented Jun 9, 2016

The PR needs updating first.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jul 13, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Aug 12, 2016

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!

Member

dmethvin commented Aug 12, 2016

@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 referenced this pull request Aug 12, 2016

Open

3.0 Deprecations and removals #970

9 of 21 tasks complete

@Krinkle Krinkle referenced this pull request Sep 20, 2016

Merged

Document jQuery.ready #983

@Krinkle

This comment has been minimized.

Show comment
Hide comment
@Krinkle

Krinkle Sep 20, 2016

Member

@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.

Member

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