-
Notifications
You must be signed in to change notification settings - Fork 10
Refactor Promise classes with generic types #24
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
Changes from all commits
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 |
---|---|---|
|
@@ -6,5 +6,3 @@ finder: | |
path: | ||
- "src" | ||
|
||
enabled: | ||
- short_array_syntax | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,9 @@ | ||
# Change Log | ||
|
||
### Added | ||
|
||
- Generic annotations | ||
|
||
## 1.1.0 - 2020-07-07 | ||
|
||
### Added | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,19 @@ | |
* A promise already fulfilled. | ||
* | ||
* @author Joel Wurtz <[email protected]> | ||
* | ||
* @template-covariant T | ||
* @implements Promise<T> | ||
*/ | ||
final class FulfilledPromise implements Promise | ||
{ | ||
/** | ||
* @var mixed | ||
* @var T | ||
*/ | ||
private $result; | ||
|
||
/** | ||
* @param $result | ||
* @param T $result | ||
*/ | ||
public function __construct($result) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
* | ||
* @author Joel Wurtz <[email protected]> | ||
* @author Márk Sági-Kazár <[email protected]> | ||
* | ||
* @template-covariant T | ||
*/ | ||
interface Promise | ||
{ | ||
|
@@ -36,10 +38,11 @@ interface Promise | |
* If you do not care about one of the cases, you can set the corresponding callable to null | ||
* The callback will be called when the value arrived and never more than once. | ||
* | ||
* @param callable|null $onFulfilled called when a response will be available | ||
* @param callable|null $onRejected called when an exception occurs | ||
* @param callable(T): V|null $onFulfilled called when a response will be available | ||
* @param callable(\Exception): V|null $onRejected called when an exception occurs | ||
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. doesn't 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. In that case, interface Promise<T> {
then<V>(onFulfilled: (value: T) => V | Promise<V>): Promise<V>;
} |
||
* | ||
* @return Promise a new resolved promise with value of the executed callback (onFulfilled / onRejected) | ||
* @return Promise<V> a new resolved promise with value of the executed callback (onFulfilled / onRejected) | ||
* @template V | ||
*/ | ||
public function then(callable $onFulfilled = null, callable $onRejected = null); | ||
|
||
|
@@ -61,7 +64,7 @@ public function getState(); | |
* | ||
* @param bool $unwrap Whether to return resolved value / throw reason or not | ||
* | ||
* @return mixed Resolved value, null if $unwrap is set to false | ||
* @return T Resolved value, null if $unwrap is set to false | ||
* | ||
* @throws \Exception the rejection reason if $unwrap is set to true and the request failed | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,9 @@ | |
* A rejected promise. | ||
* | ||
* @author Joel Wurtz <[email protected]> | ||
* | ||
* @template-covariant T | ||
* @implements Promise<T> | ||
*/ | ||
final class RejectedPromise implements Promise | ||
{ | ||
|
@@ -14,9 +17,6 @@ final class RejectedPromise implements Promise | |
*/ | ||
private $exception; | ||
|
||
/** | ||
* @param \Exception $exception | ||
*/ | ||
public function __construct(\Exception $exception) | ||
{ | ||
$this->exception = $exception; | ||
|
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.
not sure what happened, it looks like instead of rebase you added the commits from master as new commits in the branch. if you can fix it, cool, otherwise i can solve that before merging.
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.
Sorry, I had some personal stuff coming up last week and didn't get around checking this. Looks like my IDE messed up the rebase, I'm going to fix the issue. Thanks for your patience!
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.
no worries, whenver you have time
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.
@Radiergummi thank you so much for this change. Might you be in a position to rebase?
This would also really benefit our SDK Generator
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.
@dbu thinking of forking @Radiergummi's changes, rebasing and submitting a new PR. Thoughts? Or should we give this more time?
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.
glad if you want to do that. you can add the changes in a new commit and then the work of each contributor is still properly attributed.