-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
Cancellable promise #20
Changes from 4 commits
9abf531
75fbae0
8b7e0f0
d5b6f8e
77f78d9
f085287
5b6ef3f
1bced89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?php | ||
|
||
namespace React\Promise; | ||
|
||
interface CancellablePromiseInterface | ||
{ | ||
public function cancel(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,30 +2,21 @@ | |
|
||
namespace React\Promise; | ||
|
||
class Promise implements PromiseInterface | ||
class Promise implements PromiseInterface, CancellablePromiseInterface | ||
{ | ||
private $canceller; | ||
private $result; | ||
|
||
private $handlers = []; | ||
private $progressHandlers = []; | ||
|
||
public function __construct(callable $resolver) | ||
private $requiredCancelRequests = 0; | ||
private $cancelRequests = 0; | ||
|
||
public function __construct(callable $resolver, callable $canceller = null) | ||
{ | ||
try { | ||
$resolver( | ||
function ($value = null) { | ||
$this->resolve($value); | ||
}, | ||
function ($reason = null) { | ||
$this->reject($reason); | ||
}, | ||
function ($update = null) { | ||
$this->progress($update); | ||
} | ||
); | ||
} catch (\Exception $e) { | ||
$this->reject($e); | ||
} | ||
$this->canceller = $canceller; | ||
$this->call($resolver); | ||
} | ||
|
||
public function then(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null) | ||
|
@@ -34,7 +25,24 @@ public function then(callable $onFulfilled = null, callable $onRejected = null, | |
return $this->result->then($onFulfilled, $onRejected, $onProgress); | ||
} | ||
|
||
return new static($this->resolver($onFulfilled, $onRejected, $onProgress)); | ||
$this->requiredCancelRequests++; | ||
|
||
return new static($this->resolver($onFulfilled, $onRejected, $onProgress), function ($resolve, $reject, $progress) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you consider adding a check to conditionally return this new behavior vs the old behavior based on if a canceller is associated with the promise? Creating a new closure when it isn't necessary would add overhead where it isn't going to be utilized. Something like: if (!$this->canceller) {
return new static($this->resolver($onFulfilled, $onRejected, $onProgress));
}
// Return the new stuff you're adding here... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, done in 77f78d9 |
||
if (++$this->cancelRequests < $this->requiredCancelRequests) { | ||
return; | ||
} | ||
|
||
$this->cancel(); | ||
}); | ||
} | ||
|
||
public function cancel() | ||
{ | ||
if (null === $this->canceller || null !== $this->result) { | ||
return; | ||
} | ||
|
||
$this->call($this->canceller); | ||
} | ||
|
||
private function resolver(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null) | ||
|
@@ -101,4 +109,23 @@ private function settle(PromiseInterface $result) | |
|
||
$this->result = $result; | ||
} | ||
|
||
private function call(callable $callback) | ||
{ | ||
try { | ||
$callback( | ||
function ($value = null) { | ||
$this->resolve($value); | ||
}, | ||
function ($reason = null) { | ||
$this->reject($reason); | ||
}, | ||
function ($update = null) { | ||
$this->progress($update); | ||
} | ||
); | ||
} catch (\Exception $e) { | ||
$this->reject($e); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just remember to update this before tagging. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep 😎