domenic / promises-unwrapping

The ES6 promises spec, as per September 2013 TC39 meeting
1.23k stars 94 forks source link

Parameter to promise constructor #9

Closed domenic closed 10 years ago

domenic commented 10 years ago

Per Promises/A+ (see also https://github.com/promises-aplus/resolvers-spec/issues/19) and for symmetry with then, the current spec does new Promise((resolve, reject) => ...). This matches Q, RSVP, and WinJS and is symmetric with then.

The Alex Russell Way of doing promise constructors would have this be new Promise(({ resolve, reject }) => ....

I see no reason to prefer the Alex Russell Way, but it is worth ensuring we get consensus. Using this issue to track any relevant debates.

annevk commented 10 years ago

It's kinda sucky to change this part of the contract as this is exposed and implemented. Also, this does not scale towards monadic promises if ES7 does those?

domenic commented 10 years ago

It does scale, as monadic promises will not be using any new constructor parameters.

It's unfortunate that implementations already are dictating spec. I had hoped it was early enough, and there was little enough web content, that changes like the ones we're making in this repository could still be made. Will we need to spec Promise.fulfill too?

domenic commented 10 years ago

Also, I think there is a lot more code using Q, RSVP, WinJS, and others than using the behind-a-flag promise implementations of Firefox and Chrome.

annevk commented 10 years ago

So @bakulf says we can change this if we do it right away. That means though that we really need to be sure about this.

In Gecko we restricted ourselves to resolve/reject so there's no relying on others.

annevk commented 10 years ago

Post about this to es-discuss? It would be kind of a precedent too for an API to hand out callbacks rather than class objects as far as I can tell.

domenic commented 10 years ago

OK, sounds good.

I hope nobody was considering handing out class objects for the { resolve, reject } pair though. There's no need for that to have a prototype, constructor, and methods that require binding before you can pass them along!

annevk commented 10 years ago

Makes sense. (It's implemented as such today though...)

domenic commented 10 years ago

No real objections so far. http://esdiscuss.org/topic/parameter-to-promise-constructor (also CC'ed to www-dom and public-script-coord)

annevk commented 10 years ago

Patch for Gecko to just change this bit in our implementation ahead of everything else: https://bugzilla.mozilla.org/show_bug.cgi?id=911213 (It's important to settle on this even quicker as this is a breaking API change for us.)

slightlyoff commented 10 years ago

Why are we breaking this? Why change it at all? We have 2 implementations that are compatible under the old mechanism and no outside reason to modify it.

annevk commented 10 years ago

Getting rid of a useless non-constructable object seems worth it. Gecko is not shipping yet, we have a patch for this change, and so far it seems this design will get us consensus.

annevk commented 10 years ago

Seems this is in the specification now.

slightlyoff commented 10 years ago

So I do object to this. And strongly. It makes it harder to build subclasses that want to vend different capabilities to the resolver callbacks: they now have to invent an ad-hoc positional parameter convention, instead of just using a unique name on the resolver object. This was an intentional choice based on the need for subclassing. Please revert, if only to turn the object passed in into a property bag. That preserved the value without the "non constructable object" (spurious) objection.

domenic commented 10 years ago

Such ad-hoc additions do not belong on the same level as the fundamental resolve and reject operations, symmetric with then (see #22) and should go in an options object.

slightlyoff commented 10 years ago

That doesn't make any sense. The fundamental operations are likely to be extended over time: cancelation, progress notification, etc. Even if you want to demand that resolve and reject (and accept) can't be over-ridden on the object, there's no reason to prevent subclasses from doing as they please with regards to those operations

domenic commented 10 years ago

Those operation are not fundamental (i.e. return <-> resolve, throw <-> reject). They are extensions.

I don't know what you're talking about with regard to overriding. Subclasses can pass whatever they want to the resolver function; separate PromiseResolver class vs. resolve and reject functions doesn't change that.