FriendsOfSymfony / FOSRestBundle

This Bundle provides various tools to rapidly develop RESTful API's with Symfony
http://symfony.com/doc/master/bundles/FOSRestBundle/index.html
MIT License
2.79k stars 707 forks source link

Add compatibility with symfony/framework-bundle 5.4 #2330

Closed Th3Mouk closed 2 years ago

Th3Mouk commented 2 years ago

This PR follow #2332 Tests are currently failling with symfony/routing v6. In order to be compatible with Symfony framework bundle in version 5.4 without delay, we have to add a conflict rule to avoid the installation of this version, the time to add/ensure a compatibility.

mbabker commented 2 years ago

This might be a quick fix, but it's not a good fix IMO.

There are 3 classes using the routing component in this bundle, but the component isn't declared in composer.json in any meaningful way (at a minimum I'd expect it in the require-dev section). There are also no checks to gracefully error out if the component is missing (i.e. an exception in the DI extension if the component isn't available). Granted, it's a bit of a long shot that the routing component wouldn't be installed, but as is, this bundle is relying on transient dependencies to get everything working right.

This would be a good time to make a better long-term fix. If the routing component should be a required dependency, it should be added to the require section in composer.json and then that would be good enough to deal with the Symfony 6 incompatibilities until full support is added. If the routing component is an optional dependency, it should be added to the require-dev section, the conflict section updated to use a good version range (i.e. <4.4, >6 to ensure newer versions of this bundle aren't installed with Symfony 4.3 or earlier), and appropriate checks added to raise an error if someone's trying to use a routing feature and the component isn't installed.

Th3Mouk commented 2 years ago

I'm agree that it's not a definitive fix, but it will allow at short term to have a compatibility. I prefer to find another solution into this two months too. It seems not that difficult to remove some listeners/services at the condition where the symfony/routing is not present dowstream. And probably switching from xml to php format for configuration file will help a lot to do that without too much pass compilers. But I'm not certains this bundle have vocation to be installed without view and routing extra behaviors. I don't know either ATM how the Route attribute will thrown exception to end user, then it will be used without routing package. Probably adding a hard dependency to symfony/routing is the best way here.

mbabker commented 2 years ago

I'd just put "symfony/routing": "^4.4|^5.0" into the require section and call it a day. The 3 uses of the routing component in this bundle:

Th3Mouk commented 2 years ago

I come to the same conclusion, what is your recommendation ?

goetas commented 2 years ago

Closing in favor of https://github.com/FriendsOfSymfony/FOSRestBundle/pull/2340