spatie / fractalistic

A framework agnostic, developer friendly wrapper around Fractal
https://freek.dev/644-a-developer-friendly-wrapper-around-fractal
MIT License
382 stars 32 forks source link

Fractal::jsonSerialize() triggers a Type error (PHP 7.4.24) #54

Closed albertoarena closed 2 years ago

albertoarena commented 2 years ago

This change done to support PHP 8.1 triggers a Type error.

Commit: https://github.com/spatie/fractalistic/commit/4d3b8a68b6575e9b0202e842977c9c29224fd9ac#diff-9ebff18d48644c7d6e2c1006245d57ef9028099e796f76f967c8656aedadcf4bR458

Change: https://github.com/spatie/fractalistic/blob/5a89b24d3153a9c4b17cd1ed523b0a5ef9143558/src/Fractal.php#L458

Error:

TypeError
Return value of Spatie\Fractalistic\Fractal::jsonSerialize() must be an instance of Spatie\Fractalistic\mixed, array returned 

Reason:

Further details:

freekmurze commented 2 years ago

The tests seem to indicate that it's working fine. Could you submit a PR with a failing tests (and maybe also the fix)?

albertoarena commented 2 years ago

@freekmurze That occurs in PHP 7.4.

We have a few projects using spatie/laravel-fractal that are still on PHP 7.4. Our deployment scripts run composer update that used latest version 2.9.2 of spatie/fractalistic, that was released today. We noticed failures in unit tests.

Latest changes deployed as version 2.9.2 may not be compatible with PHP 7.4 due to mixed pseudo type used in public function jsonSerialize(): mixed.

I'll run a few tests in PHP 7.4 and 8.1, and create a pull request with a solution that makes spatie/fractalistic backward compatible with PHP 7.4.

albertoarena commented 2 years ago

@freekmurze I have a fix ready, tested with php 7.4 and 8.1, but I am not allowed to create a pull request.

Should I be added to contributors?

Thanks

freekmurze commented 2 years ago

Creating pull requests is not restricted in any way.

albertoarena commented 2 years ago

PR: https://github.com/spatie/fractalistic/pull/55

freekmurze commented 2 years ago

We'll handle this further in #55