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

Add catch() and finally(), deprecate otherwise() and always() #208

Merged
merged 1 commit into from
Jan 23, 2022

Conversation

clue
Copy link
Member

@clue clue commented Jan 23, 2022

This changeset adds new catch() and finally() methods to the PromiseInterface and deprecates the otherwise() and always() methods. The underlying idea is to rename otherwise() to catch() and always() to finally(), but instead of performing a hard rename and causing a BC break, this changeset proposes deprecating the old method names and using them as an alias for the new method names.

// old (deprecated)
$promise->otherwise($onRejected);
$promise->always($onFulfilledOrRejected);

// new
$promise->catch($onRejected);
$promise->finally($onFulfilledOrRejected);

The original method names have been added in #19 a while back. Back then, using the catch() and finally() method names was not possible as using reserved keywords is only supported as of PHP 7+. Promise v3 is PHP 7.1+ only (#138 / #149), so we can now use the method names that mimic a synchronous try/catch/finally block. Additionally, this also happens to be in line with ES6 promises as found in JavaScript.

Supersedes / closes #206, thanks @WyriHaximus for the original version! This PR follows the exact same idea with no functional difference, but adds additional documentation and duplicates all relevant test cases to achieve 100% code coverage for the affected code paths. Accordingly, there's a fair amount of duplication in the test suite now, which I consider a non-ideal but acceptable tradeoff. A potential promise v4 could remove this duplication at some point in the future.

@clue clue added this to the v3.0.0 milestone Jan 23, 2022
@WyriHaximus WyriHaximus self-requested a review January 23, 2022 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants