[FrameworkBundle] Deprecate setting the `collect_serializer_data` to `false` #60069

mtarld · 2025-03-28T08:46:07Z

Q	A
Branch?	7.3
Bug fix?	no
New feature?	no
Deprecations?	yes
Issues
License	MIT

Then, in 8.1, we'll be able to deprecate this option to finally remove it in 9.0

nicolas-grekas · 2025-03-28T08:53:56Z

What's the rational? Didn't we add the option because the overhead was significant? Did anything change?

mtarld · 2025-03-28T09:00:48Z

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.

mtarld · 2025-03-28T09:01:58Z

But maybe at least we can change the default? So that we can remove the recipe definition at some point?

Which means:

requiring to define explicitly the value in 7.3
change the default in 8.0

WDYT?

mtarld · 2025-03-28T10:22:51Z

Here is the PR introducing the flag: #46625

chalasr · 2025-03-28T10:35:39Z

Indeed, adding this option was a quick fix. It isn't meant to stay

mtarld · 2025-03-28T12:29:15Z

There are still few tests to fix for information

fabpot · 2025-03-28T12:29:39Z

Why not deprecate the option instead?

mtarld · 2025-03-28T13:20:13Z

Because the actual default value is false, and I want to have a true in the end, I think the steps should be:

deprecate not setting the value to true (7.3)
deprecate the config option (8.1) - as it'll be true for sure
remove the config option (9.0)

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?

chalasr · 2025-03-28T18:57:21Z

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.

mtarld · 2025-03-29T10:20:23Z

The related tests are fixed now.

nicolas-grekas · 2025-03-31T14:50:25Z

@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.

chalasr · 2025-03-31T14:54:31Z

Fair enough

nicolas-grekas · 2025-03-31T15:00:05Z

In the upgrade note, it might be nice to tell about what you wrote in #60069 (comment) BTW.

mtarld · 2025-03-31T15:29:08Z

Here you go: https://github.com/symfony/symfony/compare/fda5371b559b9c03113c726a7e33dbbd176339b3..29c75621c740dea5df27d86fff4293ae5c50b673

…`false`

nicolas-grekas · 2025-03-31T15:37:50Z

Thank you @mtarld.

…ializer_data` option (xabbuh) This PR was merged into the 7.3 branch. Discussion ---------- [FrameworkBundle] fix default value for the `collect_serializer_data` option If I don't miss anything, symfony/symfony#60069 didn't change the default value. Commits ------- 4545364 fix default value for the "collect_serializer_data" option

…t_serializer_data` (GromNaN) This PR was merged into the 7.4 branch. Discussion ---------- [FrameworkBundle] Update deprecation message for `collect_serializer_data` | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT "false" is the default value. The current deprecation message makes me think I have set `collect_serializer_data: false` in my configuration, but in fact it was not specified. That was the default configuration: https://github.com/symfony/recipes/blob/main/symfony/web-profiler-bundle/6.1/config/packages/web_profiler.yaml#L11 Related to #60069 Commits ------- c2dfe2f [FrameworkBundle] Update deprecation message for collect_serializer_data

…data` (mtarld) This PR was merged into the 8.1 branch. Discussion ---------- [FrameworkBundle] Deprecate setting `collect_serializer_data` | Q | A | ------------- | --- | Branch? | 8.1 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Issues | | License | MIT Follow-up of #60069. Deprecate setting `collect_serializer_data`, as it's not used anymore by `FrameworkBundle` Commits ------- d97d09c [FrameworkBundle] Deprecate setting `collect_serializer_data`

…data` (mtarld) This PR was merged into the 8.1 branch. Discussion ---------- [FrameworkBundle] Deprecate setting `collect_serializer_data` | Q | A | ------------- | --- | Branch? | 8.1 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Issues | | License | MIT Follow-up of symfony/symfony#60069. Deprecate setting `collect_serializer_data`, as it's not used anymore by `FrameworkBundle` Commits ------- d97d09cb1b7 [FrameworkBundle] Deprecate setting `collect_serializer_data`

…alizer_data` (mtarld) This PR was merged into the 8.1 branch. Discussion ---------- [FrameworkBundle] Deprecate setting `collect_serializer_data` | Q | A | ------------- | --- | Branch? | 8.1 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Issues | | License | MIT Follow-up of symfony#60069. Deprecate setting `collect_serializer_data`, as it's not used anymore by `FrameworkBundle` Commits ------- d97d09c [FrameworkBundle] Deprecate setting `collect_serializer_data`

carsonbot added Status: Needs Review Deprecation FrameworkBundle labels Mar 28, 2025

carsonbot added this to the 7.3 milestone Mar 28, 2025

mtarld force-pushed the feat/collect-serializer-data branch from f842425 to 5cf77b8 Compare March 28, 2025 08:56

mtarld requested a review from chalasr as a code owner March 28, 2025 08:56

chalasr approved these changes Mar 28, 2025

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 28, 2025

fabpot approved these changes Mar 28, 2025

View reviewed changes

mtarld force-pushed the feat/collect-serializer-data branch 2 times, most recently from fcf4b27 to 2f40ad4 Compare March 29, 2025 10:15

mtarld force-pushed the feat/collect-serializer-data branch from 2f40ad4 to fda5371 Compare March 31, 2025 09:03

mtarld force-pushed the feat/collect-serializer-data branch from fda5371 to 29c7562 Compare March 31, 2025 15:28

[FrameworkBundle] Deprecate setting the collect_serializer_data to …

408d09a

…`false`

mtarld force-pushed the feat/collect-serializer-data branch from 29c7562 to 408d09a Compare March 31, 2025 15:34

nicolas-grekas approved these changes Mar 31, 2025

View reviewed changes

nicolas-grekas merged commit 58a14ab into symfony:7.3 Mar 31, 2025
10 checks passed

nicolas-grekas mentioned this pull request Mar 31, 2025

[FrameworkBundle] Deprecate setting the collect_serializer_data to `f… symfony/symfony-docs#20847

Closed

mtarld deleted the feat/collect-serializer-data branch March 31, 2025 15:47

fabpot mentioned this pull request May 2, 2025

Release v7.3.0-BETA1 #60327

Merged

xabbuh mentioned this pull request May 5, 2025

[FrameworkBundle] fix default value for the collect_serializer_data option symfony/symfony-docs#20936

Merged

mtarld mentioned this pull request Jun 6, 2025

[FrameworkBundle] Deprecate setting collect_serializer_data #60725

Closed

OrestisZag mentioned this pull request Jun 17, 2025

[Serializer] Fix TraceableSerializer when called from a callable inside array_map #60809

Merged

xabbuh mentioned this pull request Jun 25, 2025

enable framework.profiler.collect_serializer_data in the test environment symfony/recipes#1433

Merged

GromNaN mentioned this pull request Nov 12, 2025

[FrameworkBundle] Update deprecation message for collect_serializer_data #62363

Merged

mtarld mentioned this pull request Nov 29, 2025

[FrameworkBundle] Deprecate setting collect_serializer_data #62559

Merged

Uh oh!

[FrameworkBundle] Deprecate setting the collect_serializer_data to false #60069

[FrameworkBundle] Deprecate setting the collect_serializer_data to false #60069

Uh oh!

Conversation

mtarld commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas commented Mar 28, 2025

Uh oh!

mtarld commented Mar 28, 2025 • edited by nicolas-grekas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtarld commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtarld commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chalasr commented Mar 28, 2025

Uh oh!

mtarld commented Mar 28, 2025

Uh oh!

fabpot commented Mar 28, 2025

Uh oh!

mtarld commented Mar 28, 2025

Uh oh!

chalasr commented Mar 28, 2025

Uh oh!

mtarld commented Mar 29, 2025

Uh oh!

nicolas-grekas commented Mar 31, 2025

Uh oh!

chalasr commented Mar 31, 2025

Uh oh!

nicolas-grekas commented Mar 31, 2025

Uh oh!

mtarld commented Mar 31, 2025

Uh oh!

nicolas-grekas commented Mar 31, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[FrameworkBundle] Deprecate setting the `collect_serializer_data` to `false` #60069

[FrameworkBundle] Deprecate setting the `collect_serializer_data` to `false` #60069

mtarld commented Mar 28, 2025 •

edited

Loading

mtarld commented Mar 28, 2025 •

edited by nicolas-grekas

Loading

mtarld commented Mar 28, 2025 •

edited

Loading

mtarld commented Mar 28, 2025 •

edited

Loading