-
Notifications
You must be signed in to change notification settings - Fork 20.6k
Improve API interoperability with standard Promise #1722
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
Comments
Author: dj.gilcrease@… Make jQuery.Defered / promise closer to conforming to the specification around exceptions. I have not checked the full compliance tests from https://github.com/promises-aplus/promises-tests but I have setup a test that shows the exception handling working as expected http://jsfiddle.net/yEXL4/1/ Pull Request @ https://github.com/jquery/jquery/pull/1462 |
Author: dmethvin I don't think this is the right way to go. You're swallowing an exception silently and it will only be reported (poorly) if the user has the foresight to attach a fail handler. See #11193, #11292. Even in the current proposed spec discussions there's some handwaving about needing browser support in dev tools to be able to debug properly. What I'd prefer to do is have it so that users can opt into standard ES6 Promises or a compliant polyfill. Retroactively changing the semantics of Deferred in a significant way seems like a bad idea, especially with no built-in support to notify about unhandled promise rejections. |
Author: markelog This ticket requires clarification. Specification that mentioned in ticket description is this one which refers to Domenic repository. There is also promises/A and promises/A+ "proposals". Which are not much of a relevance right now. Plus upcoming draft of ES6 which should also have Promise clause. DOM specification is in the draft stage, but it's already implemented in two browsers – Firefox (Nightly) and Chrome (Canary) without any prefixes, both browsers however has some implementation mistakes. New specification defines new API which significantly differs from the jQuery Deferred, aside from methods like "cast", "catch", etc; it also has these behaivor differences:
|
Author: dmethvin It's definitely good to emphasize that Promises/A+ is just a proposal and not a standard to comply with. jQuery has had Deferred for several years now and I would guess it has the most number of users of any Promise/Deferred implementation that exists today. Let's not break that code, keep Deferred the way it is, and look for ways to let developers use the emerging standard. There has been some lively discussion around the addition of a .done() method to ES6 Promises that would not swallow exceptions, but it doesn't seem to be slated for the first iteration of the standard. I think that's a shame, because .then() can be built from .done() but not the other way around. Without the proposed browser support for exposing unhandled exceptions, Promises are going to be very difficult to debug. |
Author: rwaldron Replying to markelog:
This is the Promises specification that will be in ES6 |
Author: domenic@… Promises/A+ is a standard and is the basis for the ES6 promises specification. All promises that follow the ES6 promise specification will pass all Promises/A+ tests. The ES6 promises spec is in effect a superset of the Promises/A+ spec. |
Author: anonymous
This is false. Promise.prototype.done = (onFulfilled, onRejected) { this.then(onFulfilled, onRejected).catch(e => setTimeout(() => throw e, 0)); }; |
Author: dmethvin Re-throwing an error object that has propagated to you isn't the same as letting the browser handle the error at the point where it occurred. A lot of important error context is lost, especially on older browsers (which 1.x still supports). However, that particular pull request listed above is about trying to turn Deferred into Promises/A+. We can't do that, it would break a few years worth of existing code that is already using Deferred. Nobody wants to break the web. Instead we expect to support ES6 Promises like I mentioned above. That seems like a good approach? EDIT: Clarified the subject; the reason this ticket isn't closed is because we expect to support ES6 Promises but are not changing Deferred. |
Author: domenic@… I guess it's unclear what the OP meant by
It sounds like you are saying something different from the OP, wherein you do not change/fix your implementation to match the spec, but instead somehow support real promises (unclear how). |
Author: dmethvin Reading through this, I agree it's unclear. The pull request referencing this issue was misplaced, since the OP doesn't mention changing our Deferred at all. If it becomes too confusing we can close this and open a new ticket with a more specific title. |
Author: m_gol I was at the Amsterdam meeting where it was discussed. OP says about compliance with ES6 spec. If wording is not clear, we can modify title & text of the issue but I'd prefer not to re-create one since discussion already started here. |
Author: m_gol We can discuss it in San Diego but, surprisingly, jaubourg seemed ok to switch once at least 2 stable browsers implement the new spec while keeping pipe with current semantics. I'd like to see how much code would be affected, I don't think many people relied (in the production code) on the promise throwing errors as a way of flow control. OTH, for jQuery even a small amount can be a lot.
There's a larger question of how we deprecate old APIs, e.g. in case of |
Author: calvin.metcalf@… Chrome beta implements them and Firefox beta has them out from under a flag, so 2 stable browsers will be soon. |
Author: markelog Replying to calvin.metcalf@…:
See ticket description – "so we'll wait for it to ship, unprefixed, not behind a flag, in stable browsers first" |
Author: anonymous Unwrapping thenables is also a change you should consider doing. It's in the spec and it's useful. |
Author: dmethvin I've edited the title to be more in line with what we actually plan to do for 1.12/2.2. We are not converting our internal uses of $.Deferred to the emerging Promise implementations, the semantics of the two differ too much and Promise lacks several features that $.Deferred uses, such as progress callbacks and multiple arguments. However, we do plan on accepting Promise as an input where appropriate (e.g., $.when). and ensuring that $.Deferred works as a thenable. |
Please consider when you're executing a handler to deferred. One of the thing every A+ promise does is always defer execution of handlers so: myPromise.then(function(){
log("World");
});
log("Hello "); Will always log "Hello World" on an A+ promise implementation. jQuery on the other hand will log either "Hello World" or "World Hello" sometimes based on whether or not the promise resolved before or after the This has been a steady source of bugs where developers rely on the order and are exposed to race conditions. I think fixing this mistake in the next major revision of jQuery could really help a lot of developers and help prevent race condition related bugs. |
@benjamingr The changes in #1996 bring |
@dmethvin that landing in jQuery would be awesome - does this means you'll also address I think I got confused by the issue date because it was migrated. |
You mean do we turn exceptions into rejections rather than allowing them to be uncaught? Yes, although we're working on how to report those in some reasonably visible way. It's related to the issue you opened in petkaantonov/bluebird#428, it's just too easy for programming errors to be lost and you see this problem over and over when people start working with Promises. Now imagine that someone drops a Promise-heavy piece of code in their web page that was written by a third party and has some problem. They won't know why it's just not working but has no visible errors. |
Bluebird does a pretty good job of detecting unhandled rejections (a simple heuristic is that those are promises you don't add I've recently made a survey on how people use Bluebird's Note that bluebird does more than just track the rejections (it stitches the stack traces, wraps primitives implicitly for logging and has a much bigger api etc) and you have different important considerations in jQuery (such as file size). So you might not want to implement all the debugging stuff that's in bluebird. Honestly just doing A+ compliance itself is huge and doing A+ compliance and unhandled rejection handling is crossing the line between good news for interoperability and being able to use jQuery as the main promise implementation in a frontend project. |
Just FYI, There's a good amount of tooling for native promises. asynchronous callstacks (already available)dedicated promises panel (coming soon)I think it's worth considering using the native Promise impl and falling back to the Deferred code for older browsers. Developers would end up with a more enjoyable debugging experience. |
We're not removing any If someone has a better plan for how to do this, please let us know. |
@paulirish how can a non-native promise hook into that tooling? |
@dmethvin i guess @paulirish meant that we can use native Promises if they available, then enhance/extend them with our methods. If i interpreted that right, this would create two code-paths which would increase the size and performance might suffer too (judging by https://github.com/petkaantonov/bluebird/tree/master/benchmark), although i'm not sure if that matters in browser envs or that we can preserve consistency between those routes. And, as i understand, the only reason for us to consider it is to improve debug process? |
@markelog native promises are really slow but they're still likely much much faster than jQuery promises - at least today. |
Pretty sure this is not accurate, |
@markelog I'm think you might want to run the benchmarks on that. For fun and glory you can also run Bluebird against itself with the sync (zalgo) build (that does not guarantee async operation). Bluebird's petkaantonov actually wrote the sync build just to prove that the performance benefits of not guaranteeing async execution are marginal at best. The trick is that instead of doing something native like using a So gnerally I'm pretty sure that Bluebird is at least two orders of magnitude faster than jQuery deferreds at the moment. Native promises using the microtask queue are probably at least one order of magnitude faster than jQuery - feel free to benchmark those. |
Since imp of
This is popular trick which used in many Promises libs, but you still need to wait for next iteration.
I see the conclusion not the argument which should lead to it.
If we would talk about microtasks vs macrotasks that would be relevant, but
Besides the points i made above, it doesn't really fair to compare async vs sync execution, since one should be inherently faster then the other. |
@markelog I'll try to be clearer and to the point: Bluebird promises will run circles around jQuery defererds any day even while jQuery uses a sync ("zalgo") execution model (which is thankfully being fixed). This is because bluebird promises consume less memory than jQuery deferred by using flags, less state variables and no closures. Here is a trivial microbenchmark in which no async execution is involved at all (so bluebird defers here and jQuery performs nothing asynhronous). This is not a typical load - it's a completely synchronous code: http://jsperf.com/bluebird-vs-jquery-promises Bluebird performs 10 times faster than jQuery here, and this is the kind of benchmark sync execution is supposed to shine - again, nothing is actually asynchronous here. For what it's worth even when you disable the async A+ deferral in Bluebird it is only marginally faster. |
Our main focus here is interop per the topic. If someone has perf issues and wants to use Bluebird or native Promise they can do so. I really do like native Promise but don't think the 90% of people who use the monolithic jQuery file would appreciate having the weight of two implementations just in order to use them. We have to solve the problem decently for all the platforms we support, all the way back to IE8 and Android 2.3. |
Tests are, you force benchmark.js execute your tests in async manner i.e. putting them in I really don't want to argue about this, implementation of |
and btw
It's not, or shouldn't be, benchmark.js couldn't give exact ops number for async tests, so this diff should fall on margin of error type of thing |
@dmethvin sorry for helping derail the conversation. :) Oleg understood where I was going with this and.. yes it would require a feature detect and forked codepath (yay, how jQuery!) but would likely end up adding more code complexity (boo!). I think the jQuery position on this kind of thing has been mixed in the past. Users will miss out on the debugging help from tools, but I understand it's not trivial for you to handle this otherwise. @markelog @benjamingr I carried this conversation about promises performance over here: https://plus.google.com/+PaulIrish/posts/ZBP64F2GEY4 Would love both your feedback. |
Woohoo!! When can we expect this to be in the release? |
It's already in git builds of compat (formerly 1.x) and (formerly 2.x), and will be in both 3.0 betas following the resolution of our blockers, which will be soon since this was the biggest of them. |
Awesome, thanks a ton :) |
The "Behavior Change" label was missing from this ticket - do we have more of those? We should pay attention so that all such tickets have this label applied. |
Originally reported by @jzaefferer at: http://bugs.jquery.com/ticket/14510
Discussed this at the jQuery team meeting in Amsterdam: The spec is still changing a lot (within whatwg and draft pages on github), so we'll wait for it to ship, unprefixed, not behind a flag, in stable browsers first.
Once that happened, we should change/fix our implementation to match the spec (and shipped implementation).
Can use .pipe() to continue using any jQuery-specific functionality.
The text was updated successfully, but these errors were encountered: