search

reactphp / promise

Promises/A implementation for PHP.
https://reactphp.org/promise/
MIT License
2.38k stars 146 forks source link

[PHP 8.4] Fixes for implicit nullability deprecation #258

Closed Ayesh closed 7 months ago

Ayesh commented 7 months ago

Fixes all issues that emits a deprecation notice on PHP 8.4.

See:

SimonFrings commented 7 months ago

Hey @Ayesh, thanks for bringing this up, always happy about contributions :+1:

I really like your changes and would love to see ReactPHP being compatible with PHP 8.4 before it comes out! The release of PHP 8.4 is set for November 2024, which is over half a year in the future. Thus the deprecation for implicit nullability might not be the only thing that will affect our projects when it comes to PHP 8.4 compatibility, so maybe it makes sense to wait before the PHP developer team finalizes their plans.

This won't be a problem for ReactPHP v3, as we support PHP >= 7.1, but for v1 (or v2 here in promise) this is going to be interesting. We support PHP >= 5.4 and since PHP 7.1 added support for nullable types, we can't use the ?TYPE syntax for older PHP versions, see https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated for more information. Not sure yet how we can achieve compatibility with PHP 8.4 in v1.

(Edit: I just noticed that you're probably the author of this PHP.Watch article, so you'll most definitely know what it's about 😄)

All in all I really appreciate the time and effort looking into this, but in my opinion we should wait until we have the first PHP 8.4 development version ready to run our test suite against and confirm our changes working. I would close this for now, but let's revisit this once we know what PHP 8.4 is going to look like.

Ayesh commented 7 months ago

This makes sense, thank you for your time reviewing this @SimonFrings. Hopefully we can revisit this and other potential changes in reactphp/* packages once it reaches its Feature Freeze.

Seldaek commented 7 months ago

@SimonFrings I think it'd be nice if you can patch v3 still before 8.4 is further because composer bundles react/promise and that means every 8.4 build prints a bunch of deprecation warnings whenever a composer command runs. This is quite noisy and because of that I'm in the process of cleaning up all these deprecation warnings in composer itself.

It's not urgent by any means but IMO harmless to do it in v3, and most likely as you said you can't (and I'd argue you shouldn't bother as it's a mere deprecation) fix it in v1/v2.

SimonFrings commented 6 months ago

@Seldaek Fair point, also talked with @clue about this and I can see that this should be handled upstream in our component. There already is a new pull request for this in #259 which builds on top of this one and adds PHP 8.4 to our test matrix, so looks good to me. Also Composer is a really impressive project (I use it everyday) and we're more than happy to help easing your development :+1:

We'll probably release this officially in a v3.2.0, so until then you can install the development version (or the specific merge commit) to use this in Composer.

Still have to look what this means for Promise v1 & v2, but I think this is a topic for another day.