-
Notifications
You must be signed in to change notification settings - Fork 4
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
Accessing resolve/reject and promise at the same time #23
Comments
I'm generally anti this as an implicit zeroth argument, but as what is essentially part of the constructor, I'm broadly in favor of it. You could use arrow functions whenever you didn't need access to this still. In addition, I think we should decide soon whether we are going to call that function synchronously or asynchronously so that if someone picks one of the earlier methods it either always works or always fails |
I think people are generally against calling the resolver asynchronously, which is a bit unfortunate, but sensible. @erights had a great comment outlining our options from there. I think we're probably going to end up with a synchronous promise constructor + asynchronous assimilation. So that would leave us with solution 2 and 3. If other people like the promise-as- |
I don't hate the promise-as-this idea for the factory function. On odd numbered days I like it. I like Q.promise being sync and Q(thenable) assimilation being async. The implementation of Q at https://code.google.com/p/es-lab/source/browse/trunk/src/ses/makeQ.js now does this. (It doesn't do the promise-as-this yet.) |
Since it feels like part of a constructor, as @ForbesLindesay said, I don't hate the promise-as-this idea, but I also don't really like it. If it gets the majority support here, I'll go along with it, but my preference would be to find another way. Why not make it explicitly a parameter? I know it adds to the number of params, but if we're willing to make it an implicit |
@briancavalier I kind of like it, and added it to the original post. But, it's a bit weird, because it breaks symmetry with |
I really think we'll end up with var resolve;
var promise = new Promise((resolve_) => { resolve = resolve_; }); We can effortlessly wrap it up in a convenience method: function defer() {
var resolve, reject, progress;
var promise = new Promise((_resolve, _reject, _progress) => {
resolve = _resolve;
reject = _reject;
progress = _progress;
});
return {promise: promise, resolve: resolve, progress: progress};
} I think we should just encourage library authors to provide that as a convenience method. |
I never intended the initialization function to be a parallel to |
@domenic Since we're mandating that I'm not really sure what to do about progress and onCancelled at this point, though :/. I still feel like this a bit of a slippery slope. Even though we don't see a potential 5th parameter right now, it seems hard to say whether or not in 2 months (or 6 months, or 12 months) we'll come up with something--and we will have already set the precendent for 4 params, so why not then add a 5th, and 6th, etc. My preference would be to assume that we will come up with more, and design the API to account for that now. fwiw, imho, I tend to agree with @erights that |
My inclination is to say spec that it's sync, but leave the defer function in "Notes" as a recommendation. That way people who go "But how can I access the promise at the same time as the resolver" can be directed to the notes section, but not all implementers have to build it in the same way. |
While implementing #18 as it stands, @wycats has run into the issue @erights outlined, of wanting access to both
resolve
/reject
andpromise
at the same time. This is pretty common, so let's dig into it a bit more:Solution 1: Awkwardness
Note that this only works if the resolver function is called synchronously (see #20).
Solution 2: Asyncness
If the resolver function is called asynchronously (again, see #20), you can use both at once.
Solution 3:
this
@wycats's solution was to bind the promise-to-be-returned as
this
, i.e. as an implicit zeroth argument inside the resolver function. I don't like this pattern in general, and think it's a bit unfortunate that such a pattern would require not using ES6 arrow functions. But some people like it, including one of our favorite implementers, so, meh?Solution 4: extra param
A variation on solution 3, this makes the parameter explicit instead of implicit, as suggested by @briancavalier.
The text was updated successfully, but these errors were encountered: