Enforce always passing promise to race function #207
Conversation
The race function has always supported passing an empty array of promises/values into it. And then defaulted to the behavior of returning an ever waiting promise. While this makes sense from a mathematical perspective, it doesn't from a developers' perspective. To enforce always having to pass in a promise, the first argument is now required, and any following promises are accepted using a variadic function argument.
|
FTR: this has been introduced later intentionally to be consistent with the JavaScript implementation, see #83. |
| return new Promise(function ($resolve, $reject) use ($promisesOrValues, $cancellationQueue): void { | ||
| foreach ($promisesOrValues as $promiseOrValue) { | ||
| return new Promise(function ($resolve, $reject) use ($promises, $cancellationQueue): void { | ||
| foreach ($promises as $promiseOrValue) { | ||
| $cancellationQueue->enqueue($promiseOrValue); | ||
|
|
||
| resolve($promiseOrValue) |
There was a problem hiding this comment.
| resolve($promiseOrValue) | |
| $promiseOrValue |
Passing throught resolve can be removed as the function signature now declares PromiseInterface.
Ah cool thanks, this came up during a call @clue, @SimonFrings and I had a few days ago. So figured I'd PR it and we see if we want to change it or keep it as is. |
|
@WyriHaximus good catch, looks very "promising" to me (sorry I'm not sorry) ^^ I talked with @clue about this one and it's a bit inconsistent to the other methods if we're getting rid of the array (cause they're all using it). The solution to this could be the What are your thoughts on this? |
|
@SimonFrings So that would result in this, correct?: /**
* Initiates a competitive race that allows one winner. Returns a promise which is
* resolved in the same way the first settled promise resolves.
*
* The returned promise will become **infinitely pending** if `$promisesOrValues`
* contains 0 items.
*
* @param non-empty-array<PromiseInterface> $promisesOrValues
* @return PromiseInterface
*/
function race(array $promisesOrValues): PromiseInterface
What if we change all methods to use variadic arguments? |
The
Agree this makes it more consistent again, but not sure how I feel about this then being inconsistent with ES6 promises, introducing a noticeable BC break for promise v3, and also how this affects our plans to switch from I still have to admit I like the approach of using a single fixed promise and a variadic list of promises to move this requirement to the type system instead of making it a runtime check. An empty |
Been looking into that one as well. And the various counts in several functions make it harder than it looks initially.
And that is the reason I wanted to at least try this and see how we all feel about it 😃 .
Alternatively, we can drop the first argument and only keep variadic argument in? For me this is low hanging fruit but I wanted to at least have a look at it. |
|
Taking the milestone of this PR as we discussed this yesterday and are unsure about it's future and if it makes sense to do this or not. |
The race function has always supported passing an empty array of promises/values into it. And then defaulted to the behavior of returning an ever waiting promise. While this makes sense from a mathematical perspective, it doesn't from a developers' perspective.
To enforce always having to pass in a promise, the first argument is now required, and any following promises are accepted using a variadic function argument.