Skip to content

Conversation

@clue
Copy link
Member

@clueclue commented Aug 24, 2020

The previous code works just fine on PHP 7 and older. PHP 8 adds
stricter type checks for closed socket resources, so the underlying
function now throws a TypeError. This can be avoided by first checking
if the socket resource is still valid (not closed). This works across
all PHP versions and also helps with avoiding some uneeded error
suppression operators.

This means this component now supports PHP 8 just fine 🎉

~/workspace/reactphp-socket$ docker run -it --rm -v `pwd`:/data --workdir /data php:8.0.0beta1-cli php -d memory_limit=-1 vendor/bin/phpunit PHPUnit 9.3.7 by Sebastian Bergmann and contributors. ....................................S.......................... 63 / 304 ( 20%) ..........SSS.................................................. 126 / 304 ( 41%) ............................................................... 189 / 304 ( 62%) ..........................................................S.... 252 / 304 ( 82%) .................................................... 304 / 304 (100%) Time: 00:08.058, Memory: 628.00 MB OK, but incomplete, skipped, or risky tests! Tests: 304, Assertions: 641, Skipped: 5.

This PR does not currently include PHP 8 in the Travis test matrix. PHP is scheduled to be released in November, I'll file a follow-up PR once it's available for installation on Travis.

Builds on top of #243 and #244

The previous code works just fine on PHP 7 and older. PHP 8 adds stricter type checks for closed socket resources, so the underlying function now throws a `TypeError`. This can be avoided by first checking if the socket resource is still valid (not closed). This works across all PHP versions and also helps with avoiding some uneeded error suppression operators.
@clueclue added this to the v1.6.0 milestone Aug 24, 2020
@clueclue requested review from WyriHaximus and jsorAugust 24, 2020 19:27
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@clue@jsor@WyriHaximus