FOSRestBundle: [BUG] Composer dependency is set improperly.

According to current documentation, the bundle needs a serializer and can either use JMSSerializerBundle or the Symfony Serializer

But if you have a look at the bundle code, it uses Symfony\Component\Serializer\Normalizer\NormalizerInterface now and then - thus meaning this bundle REQUIRES the Symfony Serializer

Steps to reproduce

Create a new project with a Symfony utility. Along with other bundles, the FrameworkBundle is installed. This bundle has no Symfony Serializer installed by default.
Try to install the FOSRestBundle.
Get an error saying a serializer is required.
Install JMSSerializerBundle. This bundle does not require Symfony Serializer. It only reqires FrameworkBundle, which, in turn, has no Symfony Serializer installed by default

Expectation: FOSRestBundle is now fully functional

Reality: FOSRestBundle is broken, because it heavily relies on Symfony\Component\Serializer

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 22 (12 by maintainers)

Most upvoted comments

okay, I see your point. thank you guys for being responsive.

yyaremenko on Jun 8, 2020

I don’t think documenting internal classes is worth it, it would increase the work to maintain this bundle, it would make the docs less readable and only might be usefull to a small part of our users in my opinion.

These classes are marked as @internal, if someone finds this class, I believe they can also find the @internal annotation. (And even if not, the symfony debug component will trigger a deprecation when extending such a class).

About making them final, @internal is more restrictive so no need imo.

GuilhemN on Jun 8, 2020

Well, you could do pretty much the same thing we do to extend the FormErrorHandler from the JMS serializer in FOS\RestBundle\Serializer\Normalizer\FormErrorHandler:

we decorate their service (see https://github.com/FriendsOfSymfony/FOSRestBundle/blob/fc2666edaccac914d1171c7d9f51be85e6f31c48/DependencyInjection/Compiler/JMSFormErrorHandlerPass.php#L35-L37)
and call it from FOS\RestBundle\Serializer\Normalizer\FormErrorHandler to alter its output.

GuilhemN on Jun 8, 2020

there should be either one single wrapper over final (non-extendable) SymfonySerializer-dependent class and final (non-extendable) JMSSerializerBundle-dependent class, so that the wrapper would give a single ‘point of access’ to needed functionality to a developer

The two serializers work differently, I don’t see the benefits of providing an abstraction over their normalizers/handlers, this would be complex (if even possible) for no gain. If you use the JMS Serializer, just use handlers, if you use the Symfony Serializer, use normalizers. That’s how things work in your project and adding a wrapper would even worsen DX in my opinion.

during the installation the bundle should only install those classes which work with the corresponding (installed) serializer and don’t install classes which depend on another (not installed) serializer

Composer doesn’t allow this. And anyway many libraries provide features that only work with suggested libraries (here it’s even just a library dependent implementation).

Honestly I don’t see the problem here, this class is not used by the bundle when the Symfony Serializer is not installed, you should do the same in your project.

GuilhemN on Jun 7, 2020

Well, this class is precisely meant to be used with the Symfony Serializer component. If you use JMS Serializer instead, you will probably want to use the FormErrorHandler class instead.

xabbuh on Jun 7, 2020