Enforce always passing promise to race function

[WyriHaximus]

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.

[jsor]

FTR: this has been introduced later intentionally to be consistent with the JavaScript implementation, see https://github.com/reactphp/promise/pull/83.

[WyriHaximus]

FTR: this has been introduced later intentionally to be consistent with the JavaScript implementation, see #83.

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.

[SimonFrings]

@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 non-empty-array<Type> annotation by PHPStan (Example Link).

What are your thoughts on this?

[WyriHaximus]

@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

it's a bit inconsistent to the other methods if we're getting rid of the array

What if we change all methods to use variadic arguments?

[clue]

@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

The $promiseOrValue is essentially still PromiseInterface|mixed, so the type hint would probably have to be non-empty-array<mixed>.

it's a bit inconsistent to the other methods if we're getting rid of the array

What if we change all methods to use variadic arguments?

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 array to iterable in the future.

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 race() is certainly not very useful, but at this point makes me wonder if there's much we can do about it without introducing more problems down the line?

[WyriHaximus]

@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

The $promiseOrValue is essentially still PromiseInterface|mixed, so the type hint would probably have to be non-empty-array<mixed>.

non-empty-array<PromiseInterface|mixed>?

it's a bit inconsistent to the other methods if we're getting rid of the array

What if we change all methods to use variadic arguments?

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 array to iterable in the future.

Been looking into that one as well. And the various counts in several functions make it harder than it looks initially.

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.

And that is the reason I wanted to at least try this and see how we all feel about it 😃 .

An empty race() is certainly not very useful, but at this point makes me wonder if there's much we can do about it without introducing more problems down the line?

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.

[WyriHaximus]

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.