Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[DependencyInjection] Add#[AutowireIterator] attribute and improve#[AutowireLocator]#51832

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

Merged

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedOct 3, 2023•
edited
Loading

QA
Branch?6.4
Bug fix?no
New feature?yes
Deprecations?no
Tickets-
LicenseMIT

This PR is building on#51392 to add an#[AutowireIterator] attribute and improve#[AutowireLocator].

The new#[AutowireIterator] attribute can be used to describe what#[AutowireLocator] can do, except that we get an iterator instead of a container.

And#[AutowireLocator] can now be used instead of#[TaggedLocator]:#[AutowireLocator('foo')] and done.

In order to describe that you want a list of services, we cannot use named arguments anymore so we have to pass an array now:#[AutowireLocator(['foo' => 'F', 'bar' => 'B'])] should be used instead of#[AutowireLocator(foo: 'F', bar: 'B')].

Last but not least, this adds support for nestingSubscribedService objects in the list of described services. This provides feature-parity with what we can do when implementingServiceSubscriberInterface.

I didn't deprecateTaggedIterator norTaggedLocator. We could, but maybe we should wait for 7.1?

TODO:

PS: while writing this, I realize that we may merge both tags in one, and letAutowirePass decide if it should build a locator or an iterator based on the type of the argument that has the attribute. We'd "just" need to find a name that'd work for that.

@carsonbotcarsonbot added this to the6.4 milestoneOct 3, 2023
@nicolas-grekasnicolas-grekas added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelOct 3, 2023
@nicolas-grekasnicolas-grekasforce-pushed thedi-autowire-tagged branch 2 times, most recently from17a9a15 to8ed9318CompareOctober 4, 2023 15:22
@nicolas-grekas
Copy link
MemberAuthor

PR updated and ready. Big Hurray to@kbond for the tests 🙏 🤗

chalasr reacted with heart emoji

@KerianMM
Copy link
Contributor

TaggedIterator and TaggedLocator attributes are they still use full?

@nicolas-grekas
Copy link
MemberAuthor

@KerianMM that's an open question. I see no hurry in deprecating them, I'd be fine waiting for 7.1.

@nicolas-grekasnicolas-grekasforce-pushed thedi-autowire-tagged branch 2 times, most recently from7bb9a9e to38c48efCompareOctober 5, 2023 13:34
@stof
Copy link
Member

stof commentedOct 5, 2023

Why not improving the existing attributes instead of adding new ones ?

@nicolas-grekas
Copy link
MemberAuthor

Why not improving the existing attributes instead of adding new ones ?

Because I think it's important to push theAutowire* convention. It helps discovering/remembering those autowiring-related attributes.

@chalasr
Copy link
Member

chalasr commentedOct 5, 2023•
edited
Loading

I didn't deprecate TaggedIterator nor TaggedLocator. We could, but maybe we should wait for 7.1?

👍 to do it on 7.1 and give time to people having them since 5.3. The docs should probably recommend the new ones asap though

@OskarStark
Copy link
Contributor

@OskarStark
Copy link
Contributor

Last but not least, this adds support for nesting SubscribedService objects in the list of described services. This provides feature-parity with what we can do when implementing ServiceSubscriberInterface.

Can you add a code example please? Thanks

@nicolas-grekas
Copy link
MemberAuthor

What's in ServiceSubscriberInterface:

* additionally, an array of {@see SubscribedService}'s can be returned:
*
* * [new SubscribedService('logger', Psr\Log\LoggerInterface::class)]
* * [new SubscribedService(type: Psr\Log\LoggerInterface::class, nullable: true)]
* * [new SubscribedService('http_client', HttpClientInterface::class, attributes: new Target('githubApi'))]

OskarStark reacted with thumbs up emoji

@OskarStark
Copy link
Contributor

Updated

@nicolas-grekas
Copy link
MemberAuthor

Thank you@kbond.

@nicolas-grekasnicolas-grekas merged commit95fa158 intosymfony:6.4Oct 6, 2023
@nicolas-grekasnicolas-grekas deleted the di-autowire-tagged branchOctober 6, 2023 09:42
OskarStark added a commit to symfony/symfony-docs that referenced this pull requestOct 10, 2023
…ator]` and add `#[AutowireIterator]` (OskarStark)This PR was squashed before being merged into the 6.4 branch.Discussion----------[DependencyInjection] Update  syntax for `#[AutowireLocator]` and add `#[AutowireIterator]`cc `@kbond`Waiting code merge*symfony/symfony#51832Fixes#18997Commits-------528cbda [DependencyInjection] Update  syntax for `#[AutowireLocator]` and add `#[AutowireIterator]`
"symfony/http-client-contracts":"<2.5",
"symfony/mailer":"<5.4",
"symfony/messenger":"<5.4",
"symfony/service-contracts":"<3.2",
Copy link
Contributor

@bendaviesbendaviesOct 11, 2023•
edited
Loading

Choose a reason for hiding this comment

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

@kbond why was this added? is there a way to remove this conflict?

this makes it not possible for us to upgrade to symfony 6.4 because this forces a requirement forpsr/container ^2.0, and we are still on v1 because the latest version oflaminas/laminas-servicemanager requires v1:
https://github.com/laminas/laminas-servicemanager/blob/3.23.x/composer.json#L26

this is sort of the same issue asapi-platform/core#5811

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry@kbond, tagged you as it looked like your commit.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

This is needed because the test case uses the $type argument of SubscribedService, which exists since 3.2.
PR welcome to change the test case and relax this.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks


if ($typeinstanceof SubscribedService) {
$key =$type->key ??$key;
$attributes =$type->attributes;
Copy link
Contributor

@bendaviesbendaviesOct 12, 2023•
edited
Loading

Choose a reason for hiding this comment

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

symfony/dependency-injection requires"symfony/service-contracts": "^2.5|^3.0, butattributes,nullable andtype are not defined onSubscribedService in until 3.2.
is this a mistake?

This was referencedOct 21, 2023
fabpot added a commit that referenced this pull requestMay 2, 2024
…d `#[TaggedLocator]` (GromNaN)This PR was merged into the 7.1 branch.Discussion----------[DependencyInjection] Deprecate `#[TaggedIterator]` and `#[TaggedLocator]`| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | yes| Issues        | Planned in#51832| License       | MIT`#[TaggedIterator]` and `#[TaggedLocator]` attributes are replaced by `#[AutowireIterator]` and `#[AutowireLocator]`, for naming consistency with all `#[Autowire*]` attributes.Commits-------4bc6bed Deprecate #[TaggedIterator] and #[TaggedLocator]
fabpot added a commit that referenced this pull requestJun 20, 2025
…#[TaggedLocator]` attributes (GromNaN)This PR was merged into the 8.0 branch.Discussion----------[DependencyInjection] Remove `#[TaggedIterator]` and `#[TaggedLocator]` attributes| Q             | A| ------------- | ---| Branch?       | 8.0| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | -| License       | MITThe new attributes `#[AutowireLocator]` and `#[AutowireIterator]` were introduced in Symfony 6.4 by#51392 and#51832.These replace the previous attributes `#[TaggedIterator]` and `#[TaggedLocator]` which were introduced in Symfony 5.4 by#40406 and subsequently deprecated in 7.1 by#54371Commits-------c096714 Remove TaggedIterator and TaggedLocator attributes
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@kbondkbondkbond approved these changes

@GromNaNGromNaNGromNaN left review comments

@stofstofstof left review comments

@weaverryanweaverryanweaverryan approved these changes

@chalasrchalasrchalasr approved these changes

@dunglasdunglasAwaiting requested review from dunglas

+1 more reviewer

@bendaviesbendaviesbendavies left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

DependencyInjectionFeature❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: Reviewed

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

10 participants

@nicolas-grekas@KerianMM@stof@chalasr@OskarStark@weaverryan@kbond@GromNaN@bendavies@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp