- Notifications
You must be signed in to change notification settings - Fork20.6k
Deferred: produce and consume standard thenables#1821
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Deferred.then is now aligned with the emerging standard. Objects that implementthe thenable interface will also be properly consumed.We use Q to test interoperability.Fixes #14510
To run tests, invoke `grunt default promises-aplus-tests`.
So is this good to go other than the enhancement@gibson042 suggested? I am thinking we should land it as-is and can do that part later. |
I'd also like to test in on real browsers and not in Node via jsdom; I plan |
I apologize for letting this response sit so long, and for its incompleteness, but—the perfect being the enemy of the good—have decided to post it anyway. We need to get on the same page with respect to a few dimensions of compatibility:
This PR proposes keeping all old behavior in For the record, it is possible to preserve all Thoughts? |
domenic commentedDec 1, 2014
What are " |
Functionality exposed by jQuery that is not in the spec:
|
domenic commentedDec 1, 2014
Thanks. I take it that by "the spec" you mean Promises/A+, not ES6. |
We generallydo mean ES6, but it's complicated by the fact that testing is against Promises/A+. |
domenic commentedDec 1, 2014
Hmm, yeah, if you wanted to pass the test262 tests for promises, then none of those would be feasible, since e.g. per ES6 onFulfilled/onRejected are only called with one argument, and .then only accepts two arguments, and so on. |
Directly in the suite? As far as I can tell, the ES6 draft leaves unspecified what happens in those "extended" scenarios: |
domenic commentedDec 1, 2014
Yes, directly in the suite. For example arguments are passed to onFulfilled and onRejected by the line
which notably does not allow passing more than one argument. |
That implements "the job PromiseReactionJob with parametersreaction and [one]argument" (https://people.mozilla.org/~jorendorff/es6-draft.html#sec-promisereactionjob), which is called by either atwo-argument The extra-argument versions appear to exist in an extra-specification domain, and are arguably compatible with it. At any rate, they would berequired for perfect backwards compatibility within jQuery. |
domenic commentedDec 1, 2014
The point is that the I agree jQuery cannot be backward compatible with itself and also |
It's not really surprising that any deviation from what the The Promises/A+ spec compliance is the best we can hope for here IMO. |
As discussed in the jQuery core meeting today, my goal is adhering to spec for code that colors inside the lines but using the rest of the space for maximum backwards compatibility: Spec compat:
Back compat:
Possible conflict:
So I will be putting a PR together after all. |
DragEvent is a superset of MouseEvent, so we want to fix up mouseproperties like pageX and pageY.Fixesgh-1925
Specific spec and old jQuery compatibility assertionsRefjquery#1821 (comment)
* It seems, check for html element (and previously for body element) was redundant* Simplify "return" statement* Add comment about potential errors that didn't find themselves in real life appClosesgh-1968
Deferred.then is now aligned with the emerging standard. Objects that implementthe thenable interface will also be properly consumed.We use Q to test interoperability.Fixes #14510
To run tests, invoke `grunt default promises-aplus-tests`.
100% Promise A+ compliant, smart interoperability (even Does not protect against exceptions in non- |
Would appreciate some code review too. |
... and I f***ed up the whole PR pretty badly so any help in handling this mess by some git guru would be much appreciated :/ |
This PR was essential in working out which changes we wanted to make and how to make them. So, it's not a complete waste, but since we will be landing#1996, this one can be closed. |
Up-to-date version of the patch that passes all the tests.