Skip to content
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

Add support for PHP 8.1 #528

Merged
merged 3 commits into from
Mar 7, 2022
Merged

Conversation

annuh
Copy link
Contributor

@annuh annuh commented Nov 17, 2021

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
Copy link

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 ?
Otherwise it seems to work and really looking forward to have it in master

@annuh
Copy link
Contributor Author

annuh commented Dec 15, 2021

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
Copy link

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

patrick-radius
patrick-radius previously approved these changes Jan 3, 2022
@christoph-kluge
Copy link

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: api-ecosystem-for-laravel/dingo-api#18

Nyholm
Nyholm previously approved these changes Mar 7, 2022
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im happy with this PR.

Im not sure if it is "complete" but every change is a step in the right direction.

Related to #533

@Nyholm Nyholm dismissed stale reviews from patrick-radius and themself via 060b99b March 7, 2022 19:56
@Nyholm Nyholm merged commit fc67d13 into thephpleague:master Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants