[RFC] Remove done() method

[clue]

This changeset suggests removing the done() method (not to be confused with then() which remains unchanged!). This is done (hah!) in an attempt to reduce the API surface and make the API less confusing and easier to understand. On top of this, this means our APIs are closer to ES6 promises commonly used in JavaScript (see also #219 and #220).

In particular, I consider this method and its relation to the then() method to be somewhat confusing and this has definitely attracted some bad code in the past. Most notably, this method would be used when you would want to ensure a promise is successfully handled in order to make sure we're not hiding any unhandled rejections.

I strongly believe the solution discussed in #87 is superior as it would ensure all unhandled rejections would be reported in the future Promise v3 by default. On top of this, we're transitioning towards using Fibers and coroutines with https://github.com/reactphp/async, so promises would be consumed using await() or yield for many use cases, which would throw any rejections, thus making it less likely to accidentally hide any exceptions.

Given reporting unhandled rejections as discussed in #87 is currently on the foreseeable roadmap, I see little value in having an additional done() method that has the sole purpose of forcefully halting the program in this case. Using an unclean shutdown is not particularly useful in long-running applications and is probably not what we would want to propose as a solution to handle this situation. If a forceful shutdown is desired (making up use cases at this point, but happy to hear about more relevant use cases!), this can still be implemented in userland:

// old
$browser->get($url)->done(function () {
    echo 'Successfully downloaded!';
});

// still supported
$browser->get($url)->then(function () {
    echo 'Successfully downloaded!';
}, function (Exception $e) {
    die 'Error: ' . $e->getMessage() . PHP_EOL;
});

Empirical evidence suggests this interface isn't used much in our ecosystem either, so I'd rather use the chance to clean up our API surface with the upcoming Promise v3 release.

Like #219 and #220, I'm filing this as an RFC to get more feedback on this method and to see how others feel about this. If this gets merged, I'll file a follow-up PR to deprecate this method for Promise v2 to ease upgrading.

[WyriHaximus]

@clue This makes sense, but only if we expect #222 to succeed. What are your and @jsor's thoughts on that?

[clue]

@clue This makes sense, but only if we expect #222 to succeed. What are your and @jsor's thoughts on that?

@WyriHaximus I'm not sure how #222 is related to #87, but I agree that we should expect unhandled rejections to be reported as discussed in #87. #222 is currently marked as WIP and doesn't contain much information besides the changeset, perhaps this should be addressed/discussed there instead?

I definitely expect #87 to be addressed for Promise v3 and have filed this RFC under this expectation. If this is something we agree on, my vote would be to merge this PR as suggested and address unhandled rejections in the above tickets for Promise v3 👍

[WyriHaximus]

@clue This makes sense, but only if we expect #222 to succeed. What are your and @jsor's thoughts on that?

@WyriHaximus I'm not sure how #222 is related to #87, but I agree that we should expect unhandled rejections to be reported as discussed in #87. #222 is currently marked as WIP and doesn't contain much information besides the changeset, perhaps this should be addressed/discussed there instead?

@clue #222 is currently marked as WIP because it involves lots of changes and I wanted to have something to show before requesting reviews. But both #222 and #224 have the same goal to resolve #87.

I definitely expect #87 to be addressed for Promise v3 and have filed this RFC under this expectation. If this is something we agree on, my vote would be to merge this PR as suggested and address unhandled rejections in the above tickets for Promise v3 +1

This PR will make #222 a lot easier actually 😉 . So yeah I'm in favor of it.