[DI] Deprecate autowiring-types in favor of aliases #21494

Merged
merged 1 commit into from Feb 1, 2017

Projects

Done in Lower entry bar

9 participants

@nicolas-grekas
Member
nicolas-grekas commented Feb 1, 2017 edited
Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #21351, #19970, #18040, #17783
License MIT
Doc PR symfony/symfony-docs#7445

https://github.com/symfony/symfony/pull/21494/files?w=1
This PR deprecates autowiring-types and replaces them by plain aliases.
ping @dunglas @weaverryan

Eg instead of

<service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader" public="false">
    <autowiring-type>Doctrine\Common\Annotations\Reader</autowiring-type>
</service>

just do:

<service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader" public="false" />
<service id="Doctrine\Common\Annotations\Reader" alias="annotations.reader" public="false" />
@nicolas-grekas nicolas-grekas added this to the 3.3 milestone Feb 1, 2017
@nicolas-grekas nicolas-grekas added to In Progress in Lower entry bar Feb 1, 2017
@nicolas-grekas nicolas-grekas moved from In Progress to In Review in Lower entry bar Feb 1, 2017
@weaverryan
Member

@nicolas-grekas Haha... I really like this simplification... but I don't understand how it works! :) Is there some significance to the service id matching the autowired interface? Otherwise... I would still expect that if I had 10 services for an interface (e.g. LoggerInterface)... that now it would simply appear that I would have 11 services for that interface. Clearly, I'm missing something ;)

@nicolas-grekas
Member
nicolas-grekas commented Feb 1, 2017 edited

It is very simple: if there is a service named "Foo\BarInterface", then that's the one, when looking to wire a "Foo\BarInterface" type hint.

UPGRADE-3.3.md
@@ -14,6 +14,8 @@ Debug
DependencyInjection
-------------------
+ * Autoriwing-types have been deprecated, use aliases instead.
@ogizanagi
ogizanagi Feb 1, 2017 Contributor

Shouldn't we add an example here?

@fabpot
fabpot Feb 1, 2017 Member

indeed, the example from the pr desc fi

@nicolas-grekas
nicolas-grekas Feb 1, 2017 Member

example added

@stof
Member
stof commented Feb 1, 2017

this is a very elegant solution to the problem, allowing to use third-party services to autowire a type (as your alias can point to any service), and forbidding to explicitly configure the same autowired type twice for different services (as service ids are unique).

So πŸ‘ for me.

@chalasr
Contributor
chalasr commented Feb 1, 2017

Far simpler πŸ‘

@dunglas
Member
dunglas commented Feb 1, 2017

Great! πŸ‘

@weaverryan
Member

Ha, I missed the change to AutowirePass. Of course, it makes sense! We'll need a docs issue so we can update things.

πŸ‘

- $definition = $container->findDefinition('templating');
- $definition->setAutowiringTypes(array(ComponentEngineInterface::class, FrameworkBundleEngineInterface::class));
+ $container->setAlias(ComponentEngineInterface::class, 'templating');
+ $container->setAlias(FrameworkBundleEngineInterface::class, 'templating');
@stof
stof Feb 1, 2017 Member

this should be private aliases. We don't want to keep these service names in the compiled container

</service>
+ <service id="Symfony\Component\EventDispatcher\EventDispatcherInterface" alias="event_dispatcher" />
+ <service id="Symfony\Component\EventDispatcher\EventDispatcher" alias="event_dispatcher" />
@stof
stof Feb 1, 2017 Member

these should be private aliases

- <autowiring-type>Symfony\Component\DependencyInjection\ContainerInterface</autowiring-type>
- <autowiring-type>Symfony\Component\DependencyInjection\Container</autowiring-type>
- </service>
+ <service id="service_container" synthetic="true" />
@stof
stof Feb 1, 2017 Member

shouldn't this one be removed entirely ? The component already handles it

- </service>
+ <service id="service_container" synthetic="true" />
+ <service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container" />
+ <service id="Symfony\Component\DependencyInjection\Container" alias="service_container" />
@stof
stof Feb 1, 2017 Member

these aliases should be private

</service>
+ <service id="Symfony\Component\Translation\TranslatorInterface" alias="translator.default" />
@stof
stof Feb 1, 2017 Member

the alias should be translator IMO instead, so that autowiring injects the real translator when using decoration. Otherwise, we loose the profiler integration when using autowiring and someone decorates the main translator alias rather than the private service.

And this alias should also be private (as any alias created purely for autowiring purpose anyway)

UPGRADE-3.3.md
@@ -14,6 +14,23 @@ Debug
DependencyInjection
-------------------
+ * Autoriwing-types have been deprecated, use aliases instead.
@stof
stof Feb 1, 2017 Member

typo here

UPGRADE-4.0.md
@@ -24,6 +24,23 @@ Debug
DependencyInjection
-------------------
+ * Autoriwing-types have been removed, use aliases instead.
@stof
stof Feb 1, 2017 Member

typo here

@nicolas-grekas nicolas-grekas [DI] Deprecate autowiring-types in favor of aliases
b11d391
@nicolas-grekas
Member

@stof comments addressed, thanks

@stof
Member
stof commented Feb 1, 2017

btw, my comment about using private aliases for this is also true for the documentation when writing it

@stof
stof approved these changes Feb 1, 2017 View changes
@weaverryan weaverryan referenced this pull request in symfony/symfony-docs Feb 1, 2017
Open

[DI] autowiring-types deprecated in place of aliases #7445

@weaverryan
Member

Docs issue created - I added a note on there about your (Stof) "private aliases" comment.

@fabpot
Member
fabpot commented Feb 1, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit b11d391 into symfony:master Feb 1, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@fabpot fabpot added a commit that referenced this pull request Feb 1, 2017
@fabpot fabpot feature #21494 [DI] Deprecate autowiring-types in favor of aliases (n…
…icolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Deprecate autowiring-types in favor of aliases

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #21351, #19970, ~~#18040~~, ~~#17783~~
| License       | MIT
| Doc PR        | symfony/symfony-docs#7445

https://github.com/symfony/symfony/pull/21494/files?w=1
This PR deprecates autowiring-types and replaces them by plain aliases.
ping @dunglas @weaverryan

Eg instead of
```xml
<service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader" public="false">
    <autowiring-type>Doctrine\Common\Annotations\Reader</autowiring-type>
</service>
```

just do:
```xml
<service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader" public="false" />
<service id="Doctrine\Common\Annotations\Reader" alias="annotations.reader" public="false" />
```

Commits
-------

b11d391 [DI] Deprecate autowiring-types in favor of aliases
1b28015
@nicolas-grekas nicolas-grekas moved from In Review to Done in Lower entry bar Feb 1, 2017
@theofidry
Contributor

@nicolas-grekas shouldn't this be back-ported to 2.8? Autowiring has been introduced there and IIRC there is autowiring-types there as well. It would allow people to never use autowiring-types in the first place and it could be considered as a bug rather than a feature.

@chalasr
Contributor
chalasr commented Feb 1, 2017

@theofidry from semver:

  • Minor version Y (x.Y.z | x > 0)
    It MUST be incremented if any public API functionality is marked as deprecated
@dunglas
Member
dunglas commented Feb 1, 2017 edited

@theofidry I don't think so. People using 2.8 LTS should not be annoyed with a deprecation in a patch version. It works well as is. This improvement is really a new feature.

@theofidry
Contributor

fair enough

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:di-autow-type-deprec branch Feb 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment