Skip to content
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

Parameter to promise constructor #9

Closed
domenic opened this issue Aug 26, 2013 · 16 comments
Closed

Parameter to promise constructor #9

domenic opened this issue Aug 26, 2013 · 16 comments

Comments

@domenic
Copy link
Owner

domenic commented Aug 26, 2013

Per Promises/A+ (see also promises-aplus/constructor-spec#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
Copy link
Collaborator

annevk commented Aug 27, 2013

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

domenic commented Aug 27, 2013

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

domenic commented Aug 27, 2013

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

annevk commented Aug 27, 2013

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

annevk commented Aug 30, 2013

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

domenic commented Aug 30, 2013

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

annevk commented Aug 30, 2013

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

@domenic
Copy link
Owner Author

domenic commented Aug 31, 2013

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

@annevk
Copy link
Collaborator

annevk commented Sep 2, 2013

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

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

annevk commented Sep 4, 2013

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

annevk commented Sep 5, 2013

Seems this is in the specification now.

@annevk annevk closed this as completed Sep 5, 2013
@slightlyoff
Copy link

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

domenic commented Sep 12, 2013

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

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

domenic commented Sep 12, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants