thephpleague / fractal

Output complex, flexible, AJAX/RESTful data structures.
fractal.thephpleague.com
MIT License
3.53k stars 351 forks source link

Add support for PHP 8.1 #528

Closed annuh closed 2 years ago

annuh commented 2 years ago

This fixes the following error on PHP 8.1 RC6:

Fatal error: During inheritance of JsonSerializable: Uncaught ErrorException: Return type of League\Fractal\Scope::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in vendor/league/fractal/src/Scope.php:298

Related to #526

MaxenceDupressoir commented 2 years ago

Hello, In first commit (f4f97897d351bbfd626dc1143a962395b0352d6b) you used return type mixed and then switched to ReturnTypeWillChange is there any reason to that ? You also updated properly composer.json in order to reflect php version the lib has been tested against and removed it later ? Otherwise it seems to work and really looking forward to have it in master

annuh commented 2 years ago

Hello, In first commit (f4f9789) you used return type mixed and then switched to ReturnTypeWillChange is there any reason to that ? You also updated properly composer.json in order to reflect php version the lib has been tested against and removed it later ?

#[ReturnTypeWillChange] was another work-around that worked. This way the minimal PHP version didn't need to be updated and older PHP projects can still use this library: https://github.com/thephpleague/fractal/blob/master/composer.json#L24

We are using this fork in a PHP 8.1.0 project without any issues. 🙂

MaxenceDupressoir commented 2 years ago

Ok got it, It work also for me but I'm not sure bumping up the minimal version is a bad thing as PHP 5.4 is very old now

christoph-kluge commented 2 years ago

I'm using this in production in combination with dingo-api package without any issues so far. This MR is required in order to remove dev- dependencies inside dingo-api package.

Is there anything missing before this gets merged?

Related PR: https://github.com/api-ecosystem-for-laravel/dingo-api/pull/18