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

Multi-yield for Q.async? #440

Closed
ndkrempel opened this issue Nov 27, 2013 · 23 comments
Closed

Multi-yield for Q.async? #440

ndkrempel opened this issue Nov 27, 2013 · 23 comments

Comments

@ndkrempel
Copy link

Within a Q.asynced function, could we allow:

yield [asyncOp1(), asyncOp2(), asyncOp3()];

as a convenient short-hand for:

yield Q.all([asyncOp1(), asyncOp2(), asyncOp3()]);

It seems like this is a very common operation and the proposed new syntax is intuitive and reduces line-noise considerably.

Technically, this would be backwards-incompatible as currently within a Q.asynced function, you can use x = yield y; for a non-promise value y and the value will pass through unchanged (after some hoops and jumps) to x. I'm not sure anyone does this however.

@domenic
Copy link
Collaborator

domenic commented Nov 27, 2013

I am sympathetic to how nice this is, but am not a big fan of overloaded behavior in general. Converting whatever is yielded to a promise seems conceptually cleaner to me.

One thing that seems apparent from how much people want this feature is that it would be worthwhile doing in ES7-timeframe await work, e.g. maybe

await all [asyncOp1(), asyncOp2(), asyncOp3()];

/cc @erights @lukehoban

@ndkrempel
Copy link
Author

I share your concerns about overloading; we don't want it to turn into the JavaScript Array constructor!

In this case, I can't think of a practical use for yielding a non-promise, and we could make yielding a non-promise non-Array throw an exception to avoid any potential confusion.

Having a separate keyword would be better, but since ES7 is quite a long way off, we have to thread any functionality we want through the yield keyword for the time being.

@petkaantonov
Copy link

@domenic Why would you convert everything to promise? I am pretty sure you can't await everything in C# and even more sure that yielding non-promise is a bug that you are making harder to find.

@ndkrempel
Copy link
Author

It's possible you have some value and you neither know nor care whether it's a promise or an actual value. Then yield can be applied to it to get a value in either case. Not sure how likely this would be in a well-structured program however...

@petkaantonov
Copy link

In such cases you can explicitly just .cast:

yield Q.cast(maybePromise)

@ndkrempel
Copy link
Author

Agree that requiring an explicit Q(...) or cast would be a good thing.

@domenic
Copy link
Collaborator

domenic commented Nov 27, 2013

In such cases you can explicitly just .cast.

This seems pretty reasonable to me, especially given that in Q .cast is so short, i.e.

yield Q(maybePromise);

It's still weird IMO to have that much magic packed into the type of the argument. I.e., you can yield two things, promises and arrays, and if it's arrays, we do this parallel thing...

@domenic
Copy link
Collaborator

domenic commented Nov 27, 2013

It's interesting that

yield Q.all([promiseA, promiseB, promiseC]);

is equivalent to

[yield promiseA, yield promiseB, yield promiseC];

but unfortunately

yield Q.all([asyncOp1(), asyncOp2(), asyncOp3()]);

is not equivalent to

[yield asyncOp1(), yield asyncOp2(), yield asyncOp3()];

@ndkrempel
Copy link
Author

Yes, that's a good way of putting it, and I think explains why yield Q.all([...]) is needed so much - you don't want to have to assign each promise to a local variable when it's only used once as an argument to yield!

@ndkrempel
Copy link
Author

If you prefer, you can think of yield as always taking an array, and yield x for non-array x is just a shorthand for yield [x]. This is a reasonably common convention in existing JavaScript.

Anyway, I think I've said all there is to be said on the topic of this shorthand, and I defer (no pun intended) to your better judgement!

@erights
Copy link
Collaborator

erights commented Nov 27, 2013

@ndkrempel writes

... explains why yield Q.all([...]) is needed so much ...

Is it? I don't know one way or the other and would appreciate any evidence. Thanks.

What about existing languages with await, like C#? Do they have any equivalent to an await all? If not, does their manual pattern equivalent to our "yield Q.all([...])" appear a lot? What fraction of all await statements?

@erights
Copy link
Collaborator

erights commented Nov 27, 2013

@ndkrempel
"yield Q.all([x])" would have as its value a promise for a singleton array containing the promised value of x, which is very different than "yield x". So I don't understand your shorthand suggestion.

@kriskowal
Copy link
Owner

@domenic Even the first two cases you illustrate are not equivalent if the latter promises reject before the first resolves.

@ndkrempel
Copy link
Author

@erights
yield Q.all([x]) would have (and has) as its value a singleton array containing the promised value of x, not a promise for such.

But agree that the shorthand applies to the output as well as the input (if you used the shorthand yield x you get the promised value of x back, not a singleton array with the promised value of x).

@erights
Copy link
Collaborator

erights commented Nov 27, 2013

What would you do with the array of resolved promises typically anyway? I would guess, pattern match it against an array pattern of variable definitions. In ES6:

const [a, b, c] = yield Q.all([op1(), op2(), op3()]);

If we're thinking about ES7 sugared syntax, "await all" still leaves the variables distant from their initializing expressions. How about

await const a = op1(), b = op2(), c = op3();

?

@ndkrempel
Copy link
Author

@erights The C# equivalent seems to be await Task.WhenAll(x, y, z). Note that they save on the array delimiters by having Task.WhenAll variadic, which improves readability I think.

@kriskowal
Copy link
Owner

@ndkrempel Q.all was variadic by spec, but I elected to use an array because we don’t have the spread operator yet. It is much more convenient to add the brackets than to add the Q.all.apply(Q, …) idiom.

@ndkrempel
Copy link
Author

@erights Some quick figures...

Google search for "C#" "await": 351,000 results.
Google search for "C#" "await Task.WhenAll": 22,200 results.
This is 6.3% (higher than I expected actually).

Ohloh code search for "await" (limited to C#): 6,008 results.
Ohloh code search for "await Task.WhenAll" (limited to C#): 121 results.
This is 2.0%.

Bear in mind that many users are inadvertently (or because they don't care about performance) serialising their code with x = await AsyncOp1() followed by y = await AsyncOp2(), and also that there are other idioms for parallel execution of tasks. Furthermore some of the matches for plain "await" are false positives... yes someone really has "class await".

It seems to me that it is common because it is necessary whenever you have a parallel fork point (e.g. waiting on multiple async operations), but without the annoyance of having to introduce a local variable solely to kick-off an async operation:

var p1 = asyncOp1(), p2 = asyncOp2(), p3 = asyncOp3();
var v1 = yield p1, v2 = yield p2, v3 = yield p3;

...is a bit too long-winded!

@erights
Copy link
Collaborator

erights commented Nov 27, 2013

const [v1, v2, v3] = yield Q.all([op1(), op2(), op3()]);

Isn't

@erights
Copy link
Collaborator

erights commented Nov 27, 2013

Given the numbers, i do not like the proposed change. The implicit cast is consistent with other promise patterns. Switching behaviors on an array test isn't, and is surprising.

@ndkrempel
Copy link
Author

@erights I would say it is, although more specifically the problem there is the line-noise aspect. Compare:

const [v1, v2, v3] = yield [op1(), op2(), op3()];

I admit that if Q.all were variadic, that would also help your example.

@ndkrempel
Copy link
Author

@erights As for the implicit cast, see @petkaantonov's comments. I think await nonTaskValue is not allowed in C#, and there is an argument for yield nonPromiseValue throwing an error as it would most likely indicate a bug, particularly given the dynamically typed nature of JavaScript (again see earlier discussion.) In some hypothetical case where a cast is desired, it seems to be clearer to make it explicit as yield Q(nonPromise) or yield Q(maybePromise) (actually there is no use for the first of these is there?).

@kriskowal
Copy link
Owner

@ndkrempel Returning non-promise values from then-handlers, and passing non-promise values to deferred.resolve are analogous cases to yielding a non-promise. We always use Q(valueOrPromise) to promote values to promises.

@jlongster asked for this feature some time ago. Although I wanted to make him happy then, and to make @ndkrempel happy now, I think that I will stand by the status quo. Explicit is better than implicit, at least in this case, and consistency is better than a dweomer, in the Perl sense, especially when it folds semantics in ways that are difficult to unfold.

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

No branches or pull requests

5 participants