[FrameworkBundle] Deprecate setting the collect_serializer_data to false
#60069
Conversation
|
What's the rational? Didn't we add the option because the overhead was significant? Did anything change? |
f842425 to
5cf77b8
Compare
|
Back in time, we added that flag to prevent apps that were injecting concrete implementations instead of interfaces to break when upgrading to 6.1. IIRC, there was no relation with any overhead at that time. For the record, users doing: public function __construct(ObjectNormalizer $normalizer) {}instead of: public function __construct(#[Autowire('...')] NormalizerInterface $normalizer) {}had broken app when enabling the debug. That was why we have hidden the decoration behind a flag. |
|
But maybe at least we can change the default? So that we can remove the recipe definition at some point? Which means:
WDYT? |
|
Here is the PR introducing the flag: #46625 |
|
Indeed, adding this option was a quick fix. It isn't meant to stay |
|
There are still few tests to fix for information |
|
Why not deprecate the option instead? |
|
Because the actual default value is
Deprecating the config option directly (and therefore removing it in 8.0) will not allow users to adapt their code first. But it may be ok for such a change. WDYT? |
|
Deprecating the option sounds good to me. The notice is what will actually urge people to fix their code so that it works with the serializer collector, no matter if it's about changing the default or removing that option. I think the latter gives enough time, no need for more. |
fcf4b27 to
2f40ad4
Compare
|
The related tests are fixed now. |
2f40ad4 to
fda5371
Compare
|
@chalasr if you mean deprecating the option whatever the value, then I don't agree: this would annoy people that just updated their recipe / created a new project since 6.1, while at the same time create a migration risk, with a leaky BC layer. |
|
Fair enough |
|
In the upgrade note, it might be nice to tell about what you wrote in #60069 (comment) BTW. |
fda5371 to
29c7562
Compare
29c7562 to
408d09a
Compare
|
Thank you @mtarld. |
Then, in 8.1, we'll be able to deprecate this option to finally remove it in 9.0