2.x returned promise types

[downsider]

I have encountered a type error when calling then() on an ExtendedPromiseInterface

In short, then() states it returns a PromiseInterface as it is inherited from that interface. However, this means that we cannot chain calls to the extended methods such as done() or otherwise() without type casting back to ExtendedPromiseInterface.

I know this is fixed in master, as both interfaces have been merged into one, but there is a trivial fix for the 2.x branch; namely changing PromiseInterface::then() to return static. Could we add this in please?

[clue]

@downsider Thanks for reporting, I understand where you're coming from. This looks like a potential duplicate, have you seen https://github.com/reactphp/promise/pull/137?

I agree that the current situation is not exactly ideal in the 2.x version, that's why we've fixed this issue for the upcoming version 3. I don't see anything that we could change about this in the 2.x version without introducing a BC break.

While this is not exactly ideal in the current version, it's not really a type error in this library. It just means that consumer of this package can not infer the return type of the then() method (and family) is an ExtendedPromiseInterface. Practically, any implementation of these interfaces inside this package is always guaranteed to return this type.

This isn't really an issue with dynamic typed languages such as PHP, as it will work out its type at runtime. This is however something that static analysis tools will report (for good reasons). This can usually be avoided by adding either additional annotations for your static analysis tools or (what I prefer) by adding assertions to your code like this:

function foo(): ExtendedPromiseInterface
{
    // …
}

$promise = foo();

$promiseChild = $promise->then(function ($result) {
    var_dump($result);
});

// if promise is an ExtendedPromiseInterface, then promiseChild should be too
assert($promiseChild instanceof ExtendedPromiseInterface);
$promiseChild->done();

This is the long form of the type-unsafe version:

$promise = foo();

$promise->then(function ($result) {
    var_dump($result);
})->done();

I believe this has been answered, so I'm closing this for now. Please come back with more details if this problem persists and we can reopen this :+1:

[downsider]

Ah, I had only searched issues rather than MRs, although the MR itself is not quite what I'm proposing so I'm hesitant to call this a duplicate.

Indeed, this does only affect static analysis tools but if you use @return static then all behaviour is preserved, is it not? I'm curious as to why you consider this to be breaking. In what scenario would a break happen?

My understanding is that this change would be updating documentation; from a code perspective, nothing is going to break because we don't enforce the return type. From a static analysis perspective, all that will have changed is that the documentation will be reporting the existing functionality, which would make this a bugfix rather than a breaking change

[clue]

The @return static essentially boils down to this: ExtendedPromiseInterface::then() must return an ExtendedPromiseInterface, whereas it is currently technically allowed to return the base PromiseInterface.

Practically, I don't think this would have a large effect, but it's still a subtle BC break anyway. This package has some 30+ Mio downloads (https://packagist.org/packages/react/promise/stats) and plenty of projects depending on it (http://packanalyst.com/class?q=React%5CPromise%5CExtendedPromiseInterface), so it's not a risk I would feel comfortable with.