Problem/Motivation

Over in #3161889: [META] Symfony 6 compatibility we have discovered that Symfony has tightened up the definition of a value in a ParameterBag:

public function set(string $name, array|bool|string|int|float|null $value) {
  $this->parameters[$name] = $value;
}

ie. the value cannot be an object.

The only place we rely on this in core is a single test case that checks that our container dumper correctly stores service references as container parameters:

        [
          ['reference' => new Reference('referenced_service')],
          ['reference' => $this->getServiceCall('referenced_service')],
          TRUE,
        ],

getServiceCall() returns an object, which can no longer be stored in a Symfony 6 ParameterBag.

Symfony's PHP dumper does not support service references as parameters; https://github.com/symfony/dependency-injection/blob/6c566c7b29e36daa806...

Steps to reproduce

Proposed resolution

Drop support for service references as parameters.

Remaining tasks

Decide if we need a deprecation notice for this.
Decide whether to throw an explicit exception or just let it fail.

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 3262853-4.patch2.08 KBlongwave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

Title: [Symfony 6] Fix support for services as container parameters » [Symfony 6] Drop support for services as container parameters
longwave’s picture

Priority: Normal » Critical
Issue tags: +Drupal 10, +Symfony 6
Parent issue: » #3161889: [META] Symfony 6 compatibility
longwave’s picture

Status: Active » Needs review
FileSize
2.08 KB

Please credit @daffie for part of this patch, taken from #3161889: [META] Symfony 6 compatibility.

daffie’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

  • catch committed 8b09bcf on 10.0.x
    Issue #3262853 by longwave, daffie: [Symfony 6] Drop support for...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 8b09bcf and pushed to 10.0.x. Thanks!

xjm’s picture

Should this have a CR?

catch’s picture

@xjm this ones OK, we were trying to support something that would never work in practice. The test coverage asserts that things are dumped, but not that you can actually do this in a functioning way.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.