-
-
Notifications
You must be signed in to change notification settings - Fork 17
Support promise cancellation #18
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
Conversation
|
👍 For the record (not suitable right now because it'd be a BC), i recommend to not typehint against namespace React\Promise\Timer;
use React\EventLoop\LoopInterface;
use React\Promise;
function timeout($input, $time, LoopInterface $loop)
{
$promise = Promise\resolve($input);
return new Promise\Promise(function ($resolve, $reject) use ($loop, $time, $promise) {
$timer = $loop->addTimer($time, function () use ($time, $promise, $reject) {
$reject(new TimeoutException($time, 'Timed out after ' . $time . ' seconds'));
$promise->cancel();
});
$promise->then(function ($v) use ($timer, $loop, $resolve) {
$loop->cancelTimer($timer);
$resolve($v);
}, function ($v) use ($timer, $loop, $reject) {
$loop->cancelTimer($timer);
$reject($v);
});
}, array($promise, 'cancel'));
}The advantages are, that you can remove a few |
|
My, bad. I see now that you're doing the |
|
I'd like to get in reactphp/promise#48 first so that we can rely on cancellation support to be present in all installations and hence make this feature more predictable. An update to this PR is ready, I'll push this once the other PR is in and a v1.2.0 has been tagged. |
Cancellation support has been backported from promise v2.1 to v1.2, so we can now rely on cancellation support being available for both major versions.
The linked PR is now in and I've updated this PR to make this feature more consistently available. Ready for review |
|
LGTM |
|
|
resolve()andreject()RuntimeExceptiontimeout()timeout()is not affectedCloses #3, supersedes/closes #13