FriendsOfSymfony / FOSJsRoutingBundle

A pretty nice way to expose your Symfony routing to client applications.
1.48k stars 261 forks source link

Allow Symfony 6 #408

Closed Kocal closed 2 years ago

Kocal commented 3 years ago
stof commented 3 years ago

Careful about that, as we should avoid releasing a version advocating support for Symfony 6 until the RC releases IMO (as the list of BC breaks is not final yet)

sanderdlm commented 2 years ago

Friendly reminder :slightly_smiling_face: Anything holding this back from being merged?

Kocal commented 2 years ago

Hum, it looks like I can't type methods to make Symfony 6 happy without breaking Symfony <= 4.. should we drop support of Symfony 3.3 and 4?

thePanz commented 2 years ago

@Kocal how is the DoctrineBundle doing in that case? it still supports those versions of SF :thinking: see: https://github.com/doctrine/DoctrineBundle/blob/2.6.x/composer.json

Kocal commented 2 years ago

@thePanz AFAIK they don't have to implement a SerializerInterface or any interfaces that changed between SF 3.x and 6.x.

Before a223ee7263ae0e5f2baa6a76cfc356269ed48250, I have the following error with Symfony 6 build:

Declaration of FOS\JsRoutingBundle\Serializer\Normalizer\RoutesResponseNormalizer::normalize($data, $format = null, array $context = []) must be compatible with Symfony\Component\Serializer\Normalizer\NormalizerInterface::normalize(mixed $object, ?string $format = null, array $context = []): ArrayObject|array|string|int|float|bool|null in /home/travis/build/FriendsOfSymfony/FOSJsRoutingBundle/Serializer/Normalizer/RoutesResponseNormalizer.php on line 25

And after a223ee7263ae0e5f2baa6a76cfc356269ed48250, I have the following error with Symfony 3.4/4.4/5.x builds:

Declaration of FOS\JsRoutingBundle\Serializer\Normalizer\RoutesResponseNormalizer::normalize($data, ?string $format = NULL, array $context = Array): array must be compatible with Symfony\Component\Serializer\Normalizer\NormalizerInterface::normalize($object, $format = NULL, array $context = Array) in /home/travis/build/FriendsOfSymfony/FOSJsRoutingBundle/Serializer/Normalizer/RoutesResponseNormalizer.php on line 20

Maybe the solution is to create a custom NormalizerInterface that will be dynamic in function of the Symfony version?

dmaicher commented 2 years ago

Actually Serializer return types have been relaxed after releasing 6.0 here.

So this will be part of 6.0.1.

Can you test if you can make it work with latest 6.0.x-dev?

sebheitzmann commented 2 years ago

can you allow also PHP8.1 ?

mhpcc commented 2 years ago

php 8.1 is allowed (^8.0), it just has to be added to travis

sebheitzmann commented 2 years ago

php 8.1 is allowed (^8.0), it just has to be added to travis

Your right, i'm sorry. Any date for the sf6 support ?

Chris53897 commented 2 years ago

Maybe a similar solution to this is possible for symfony 4.4 support. https://github.com/briannesbitt/Carbon/issues/2508#issuecomment-983756110

In my eyes a new major version php8 only with support for symfony 5.4 and 6.x should be created to lower the burden of the maintainers.

sebheitzmann commented 2 years ago

Could you create a 3.0 branch with a sf6 support ? This bundle is the last one blocking my project upgrade.

sanderdlm commented 2 years ago

@Kocal The two remaining errors:

Fatal error:  Declaration of FOS\JsRoutingBundle\Serializer\Denormalizer\RouteCollectionDenormalizer::supportsDenormalization($data, string $type, ?string $format = NULL): bool must be compatible with Symfony\Component\Serializer\Normalizer\DenormalizerInterface::supportsDenormalization($data, $type, $format = NULL) in /home/travis/build/FriendsOfSymfony/FOSJsRoutingBundle/Serializer/Denormalizer/RouteCollectionDenormalizer.php on line 18

Fatal error: Declaration of FOS\JsRoutingBundle\Serializer\Denormalizer\RouteCollectionDenormalizer::supportsDenormalization($data, string $type, ?string $format = NULL): bool must be compatible with Symfony\Component\Serializer\Normalizer\DenormalizerInterface::supportsDenormalization($data, $type, $format = NULL) in /home/travis/build/FriendsOfSymfony/FOSJsRoutingBundle/Serializer/Denormalizer/RouteCollectionDenormalizer.php on line 18

seem to be related to this change you introduced in a223ee7263ae0e5f2baa6a76cfc356269ed48250. You've reverted some of these changes in 40108d5395c8b4f380ced83d9bafb71ea5fe5210, but not the supportsDenormalization bool return type. Could you also revert the return type on that method? It is also reverted in the Symfony PR that eased up the return types. Afterwards, we wait for Symfony 6.0.1 I guess :slightly_smiling_face:

Bukashk0zzz commented 2 years ago

And Symfony 6.0.1 is here :) Any updates?

mhpcc commented 2 years ago

symfony 6.0.1 has not removed the return type declarations from serializer but the ones in the normalizer interfaces, so this should be ready to be merged

tobias-93 commented 2 years ago

Hi,

Sorry, this bundle has been under my radar for a long time. Please rebase on the newest master to test this on all dependencies and PHP versions we're currently supporting (using Github Actions). I'll review it when you've done this so we can finally support SF6.

Kocal commented 2 years ago

Hi @tobias-93, no worries! :)

The PR has been rebased on latest master. I've removed Symfony 3.4 and 5.0/5.1/5.2 support since those versions are not maintened anymore.

I would still need help with normalizers params/returns types conflicts.

Kocal commented 2 years ago

The CI is green, I've literally removed types on (de)normalizers... :grimacing:

I think we can add them back when dropping support of Symfony 4.4 or/and 5.3.

sebheitzmann commented 2 years ago

Good news. Congrat

stof commented 2 years ago

The way to be compatible with all versions is to support only PHP 7.2+ (PHP 7.1 does not support variance rules) and to add the return type in your class (but not the argument type). This way, you are compatible with both versions thanks to variance rules.

tobias-93 commented 2 years ago

Thanks @Kocal