NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:masterfromnicolas-grekas:di-autow-type-deprec

Feb 1, 2017

Merged

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

fabpot merged 1 commit intosymfony:masterfromnicolas-grekas:di-autow-type-deprec

Feb 1, 2017

Conversation

Copy link

Member

nicolas-grekas commentedFeb 1, 2017•
edited by weaverryan
Loading

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

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

just do:

<serviceid="annotations.reader"class="Doctrine\Common\Annotations\AnnotationReader"public="false" /><serviceid="Doctrine\Common\Annotations\Reader"alias="annotations.reader"public="false" />

nicolas-grekas added this to the3.3 milestone

Feb 1, 2017

carsonbot added Status: Needs Review DependencyInjection Feature Deprecation labels

Feb 1, 2017

nicolas-grekas force-pushed thedi-autow-type-deprec branch from4366566 toc3e73dbCompare

February 1, 2017 14:59

Copy link

Member

weaverryan commentedFeb 1, 2017

@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 have11 services for that interface. Clearly, I'm missing something ;)

Copy link

MemberAuthor

nicolas-grekas commentedFeb 1, 2017•
edited
Loading

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.

ogizanagi reviewed

Feb 1, 2017

View reviewed changes

UPGRADE-3.3.md Outdated

		DependencyInjection
		-------------------

		* Autoriwing-types have been deprecated, use aliases instead.

Copy link

Contributor

ogizanagiFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Shouldn't we add an example here?

Copy link

Member

fabpotFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

indeed, the example from the pr desc fi

Copy link

MemberAuthor

nicolas-grekasFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

example added

Copy link

Member

stof commentedFeb 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.

nicolas-grekas force-pushed thedi-autow-type-deprec branch fromc3e73db to78c9141Compare

February 1, 2017 16:33

Copy link

Member

chalasr commentedFeb 1, 2017

Far simpler 👍

Copy link

Member

dunglas commentedFeb 1, 2017

Great! 👍

Copy link

Member

weaverryan commentedFeb 1, 2017

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

👍

stof requested changes

Feb 1, 2017

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php Outdated

		$definition =$container->findDefinition('templating');
		$definition->setAutowiringTypes(array(ComponentEngineInterface::class,FrameworkBundleEngineInterface::class));
		$container->setAlias(ComponentEngineInterface::class,'templating');
		$container->setAlias(FrameworkBundleEngineInterface::class,'templating');

Copy link

Member

stofFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml Outdated

		<autowiring-type>Symfony\Component\EventDispatcher\EventDispatcher</autowiring-type>
		</service>
		<serviceid="Symfony\Component\EventDispatcher\EventDispatcherInterface"alias="event_dispatcher" />
		<serviceid="Symfony\Component\EventDispatcher\EventDispatcher"alias="event_dispatcher" />

Copy link

Member

stofFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

these should be private aliases

src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml Outdated

		<autowiring-type>Symfony\Component\DependencyInjection\ContainerInterface</autowiring-type>
		<autowiring-type>Symfony\Component\DependencyInjection\Container</autowiring-type>
		</service>
		<serviceid="service_container"synthetic="true" />

Copy link

Member

stofFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml Outdated

		</service>
		<serviceid="service_container"synthetic="true" />
		<serviceid="Symfony\Component\DependencyInjection\ContainerInterface"alias="service_container" />
		<serviceid="Symfony\Component\DependencyInjection\Container"alias="service_container" />

Copy link

Member

stofFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

these aliases should be private

src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml Outdated


		<autowiring-type>Symfony\Component\Translation\TranslatorInterface</autowiring-type>
		</service>
		<serviceid="Symfony\Component\Translation\TranslatorInterface"alias="translator.default" />

Copy link

Member

stofFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

the alias should betranslator 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 maintranslator 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 Outdated

		DependencyInjection
		-------------------

		* Autoriwing-types have been deprecated, use aliases instead.

Copy link

Member

stofFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

typo here

UPGRADE-4.0.md Outdated

		DependencyInjection
		-------------------

		* Autoriwing-types have been removed, use aliases instead.

Copy link

Member

stofFeb 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

typo here

[DI] Deprecate autowiring-types in favor of aliases

b11d391

nicolas-grekas force-pushed thedi-autow-type-deprec branch from78c9141 tob11d391Compare

February 1, 2017 18:43

Copy link

MemberAuthor

nicolas-grekas commentedFeb 1, 2017

@stof comments addressed, thanks

Copy link

Member

stof commentedFeb 1, 2017

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

stof approved these changes

Feb 1, 2017

View reviewed changes

weaverryan mentioned this pull request

Feb 1, 2017

[DI] autowiring-types deprecated in place of aliasessymfony/symfony-docs#7445

Closed

Copy link

Member

weaverryan commentedFeb 1, 2017

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

Copy link

Member

fabpot commentedFeb 1, 2017

Thank you@nicolas-grekas.

fabpot merged commitb11d391 intosymfony:master

Feb 1, 2017

fabpot added a commit that referenced this pull request

Feb 1, 2017

feature#21494[DI] Deprecate autowiring-types in favor of aliases (n…

1b28015

…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#7445https://github.com/symfony/symfony/pull/21494/files?w=1This PR deprecates autowiring-types and replaces them by plain aliases.ping@dunglas@weaverryanEg instead of```xml<service public="false">    <autowiring-type>Doctrine\Common\Annotations\Reader</autowiring-type></service>```just do:```xml<service public="false" /><service alias="annotations.reader" public="false" />```Commits-------b11d391 [DI] Deprecate autowiring-types in favor of aliases

nicolas-grekas mentioned this pull request

Feb 1, 2017

[Autowiring] Add interface FQCN as a valid service name to avoid collisions#21132

Closed

Copy link

Contributor

theofidry commentedFeb 1, 2017

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

Copy link

Member

chalasr commentedFeb 1, 2017

@theofidry fromsemver:

Minor version Y (x.Y.z | x > 0)
It MUST be incremented if any public API functionality is marked as deprecated

Copy link

Member

dunglas commentedFeb 1, 2017•
edited
Loading

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

Copy link

Contributor

theofidry commentedFeb 1, 2017

fair enough

nicolas-grekas deleted the di-autow-type-deprec branch

February 1, 2017 23:23

nicolas-grekas mentioned this pull request

Feb 2, 2017

[DI] Implement PSR-11#21265

Merged

5 tasks

nicolas-grekas mentioned this pull request

Feb 14, 2017

[FrameworkBundle] Support autowiring for RouterInterface#19972

Closed

Copy link

Contributor

sstok commentedFeb 21, 2017•
edited
Loading

```Using the attribute "class" is deprecated for the service "2_d44d409a6f8ec9217d33bfd752130a5a50dfdd4314933b690caf9278e3ece37c" which is defined as an alias in "/[..]/rollerworks/search-bundle/src/DependencyInjection/../Resources/config/search_processor.xml". Allowed attributes for service aliases are "alias", "id" and "public". The XmlFileLoader will raise an exception in Symfony 4.0, instead of silently ignoring unsupported attributes: 1x    1x in SearchProcessorTest::testEmptySearchCodeIsValid from Rollerworks\Bundle\SearchBundle\Tests\Functional```Never mind 🙈 class !== id 😅

Copy link

Member

chalasr commentedFeb 21, 2017

@sstok Using public aliases works fine on my side. The exception message make me think it's unrelated. Maybe open an issue showing the failing service definition (fromsearch_processor.xml)?