Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Closed
jaubourg wants to merge40 commits intomasterfromstandard-then-tests-fixed

Conversation

jaubourg
Copy link
Member

Up-to-date version of the patch that passes all the tests.

@dmethvin
Copy link
Member

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.

@mgol
Copy link
Member

I'd also like to test in on real browsers and not in Node via jsdom; I plan
to do it. It can be done later, though, I don't want to delay it further.

@gibson042
Copy link
Member

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:

  • Backwards
    • Do we remove our.then extensions?
    • Do we change the meaning ofnew jQuery.Deferred().resolve( thenable )?
  • Forwards
    • Do we undeprecate.pipe?
  • Spec
    • Do we let deferreds break irreparably on code likedfd.done( thrower ).then( neverCalled, neverCalled ); dfd.resolve()?

This PR proposes keeping all old behavior in.pipe and reducing.then to a by-the-spec implementation. After digesting it, I think more conversation is merited on all of the above points before accepting anything. I've putmy work on hold to address them.

For the record, it is possible to preserve all.then extensions with the possible exception of context-setting. I'd personally like to do so, perhaps with deprecation, remove the already-deprecated.pipe, and fix the "broken forever" case.

Thoughts?

@domenic
Copy link

What are ".then extensions"?

@gibson042
Copy link
Member

Functionality exposed by jQuery that is not in the spec:

  • Multi-valued fulfillment and rejection
  • Contextual (this-providing) fulfillment and rejection
  • Progress callbacks

@domenic
Copy link

Thanks. I take it that by "the spec" you mean Promises/A+, not ES6.

@gibson042
Copy link
Member

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
Copy link

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.

@gibson042
Copy link
Member

@domenic
Copy link

Yes, directly in the suite. For example arguments are passed to onFulfilled and onRejected by the line

Let status be the result of calling the [[Call]] internal method of promiseCapability.[[Reject]] passing undefined as thisArgument and (handlerResult.[[value]]) as argumentsList.

which notably does not allow passing more than one argument.

@gibson042
Copy link
Member

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-argumentthen or single-valuedfulfill/reject.

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
Copy link

The point is that thethen method is specified as only having two arguments, and any other arguments must be ignored. They certainly can't change the behavior ofthen to be something different than specified.

I agree jQuery cannot be backward compatible with itself and alsocompatiblecompliant with ES6.

@mgol
Copy link
Member

mgol commentedDec 1, 2014

It's not really surprising that any deviation from what thePromise object in browsers does makes a particular implementation incompatible. After all, ES6 goal is to specify everything in enough detail that any differences between implementations are unobservable. jQuery Deferreds have to be distinguishable from ES6Promise for back compat so perfect compliance is impossible.

The Promises/A+ spec compliance is the best we can hope for here IMO.

@gibson042
Copy link
Member

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:

  • Don't swallow resolution:dfd.done( thrower ).then( expectsInvocation ); dfd.resolve()

Back compat:

  • Don't process resolution values:dfd.resolve( thenable ).done( expectsThenable )
  • Multi-valued resolution:dfd.resolve( value1, value2, ... ).then( expectsMultipleArguments )
  • Progress:dfd.notify( progress ).then( null, null, expectsInvocation )

Possible conflict:

  • Contextual resolution:dfd.resolveWith( context, values ).then( expectsContext )

So I will be putting a PR together after all.

mgoland others added3 commitsJanuary 3, 2015 19:42
DragEvent is a superset of MouseEvent, so we want to fix up mouseproperties like pageX and pageY.Fixesgh-1925
gibson042 added a commit to gibson042/jquery that referenced this pull requestJan 6, 2015
Specific spec and old jQuery compatibility assertionsRefjquery#1821 (comment)
(cherry picked from commit18baae2)
not present: `< 0`present: `> -1`at index: `=== N`Closesgh-1984
gibson042and others added18 commitsJanuary 11, 2015 10:28
* 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`.
@jaubourg
Copy link
MemberAuthor

   raw     gz Compared to master @ d7e5fcee519e5f3e840beef9e67a536e75133df9 +1769   +478 dist/jquery.js  +629   +235 dist/jquery.min.js

100% Promise A+ compliant, smart interoperability (even.pipe() can handle standard promises).

Does not protect against exceptions in non-then handlers like#1996, which does so at the cost of nearly doubling memory usage (not worth it imo).

@jaubourg
Copy link
MemberAuthor

Would appreciate some code review too.

@jaubourg
Copy link
MemberAuthor

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

@timmywil
Copy link
Member

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.

@timmywiltimmywil deleted the standard-then-tests-fixed branchApril 29, 2015 22:11
@locklockbot locked asresolvedand limited conversation to collaboratorsJan 19, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

17 participants
@jaubourg@dmethvin@mgol@gibson042@domenic@timmywil@EvanCarroll@scottgonzalez@markelog@sakiss@MerMalibu@jquerybot@agcolom@araghava@victor-homyakov@togakangaroo@LeonardoBraga

[8]ページ先頭

©2009-2025 Movatter.jp