Make nullable parameter types explicit#34
Conversation
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
thanks for looking into this. some questions in code comments
|
|
||
| ## 1.3.1 - unreleased | ||
|
|
||
| - Made nullable parameter types explicit (PHP 8.4 compatibility) |
There was a problem hiding this comment.
is this going to be a BC break of PHP 8.4, or only become deprecated?
There was a problem hiding this comment.
PHP 8.4 triggers a E_DEPRECATED error when it loads a file with this kind of parameter type declaration.
| * @return Promise a new resolved promise with value of the executed callback (onFulfilled / onRejected) | ||
| */ | ||
| public function then(callable $onFulfilled = null, callable $onRejected = null); | ||
| public function then(?callable $onFulfilled = null, ?callable $onRejected = null); |
There was a problem hiding this comment.
afaik this is a BC break for anything implementing the promise, right?
if it is, we should bump the major version number.
There was a problem hiding this comment.
No, the code is 100% equivalent. This has always been a nullable type.
There was a problem hiding this comment.
Here's an example where I examine the type with reflection:
One parameter is declared callable, the other ?callable. As you can see in the output, both parameters look the same.
There was a problem hiding this comment.
thanks. i also added a class that implements the interface and changes ?
at least up to 8.3 this is indeed equivalent.
thanks for the clarification, then we can savely merge this.
|
Thank you! |
|
@dbu, would you be open to me porting the same fix to the 1.2.x release? We're blocked on the same issue. |
|
@Ndiritu Is there anything in 1.3.x that is blocking from updating to it but rather stay on 1.2.x instead? |
|
@xabbuh 1.3.x deprecated generic annotations which our library depends on. Stable support for generics would help us upgrade. |
|
deprecating means it will be removed in the next major version. we aim to not do any BC breaks in minor versions. a deprecation is a warning, not an error. if you have your CI report them, configure it to accept those deprecations for now to be able to use the latest version. |
4 participants
What's in this PR?
This PR turns implicit nullable parameter types to explicit ones.
Why?
PHP allows to implicitly declare a nullable parameter type by setting the parameter's default value to
null. However, in PHP 8.4 this syntax has been deprecated.https://wiki.php.net/rfc/deprecate-implicitly-nullable-types