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

[DI] Allow binding by type+name#27165

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
nicolas-grekas merged 1 commit intosymfony:masterfromnicolas-grekas:bind-tuple
May 21, 2018

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

This would allow to bind by type + argument name, e.g.:

bind:Psr\Log\LoggerInterface $logger:@logger

Allows more precise targets for bindings as it will match only if both the type and the name match.
Works with scalar/array types also for consistency.

ro0NL, jvasseur, ogizanagi, yceruto, linaori, fsevestre, kbond, ismail1432, apfelbox, and regniblod reacted with thumbs up emojiKoc reacted with hooray emojiismail1432, AlexDpy, and SimonHeimberg reacted with heart emoji
@ogizanagi
Copy link
Contributor

ogizanagi commentedMay 5, 2018
edited
Loading

I guess that's what you had in mind for solving#27054 differently?
Not sure how I feel between changing the typehint vs the constructor argument name for such cases.

Your solution requires less work (no extra class to create & register) but I think is also more error prone (naming the argument against the right name to get the right bus). You might end up using the wrong one without any warning.

NVM..., this is not actually required for the mentioned issue... Just regular binding does.

(but anyway I'm 👍 for this)

}

if (isset($key[0]) &&'$' ===$key[0]) {
if (preg_match('/^(?:(?:array|bool|float|int|string) )?\$/',$key)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Anditerable?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

This needs to contain what we can put in parameters. So not iterable nor callable.

Copy link
Contributor

@ogizanagiogizanagiMay 6, 2018
edited
Loading

Choose a reason for hiding this comment

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

Theoretically, a factory definition could return a callable or iterable (or any other type) and be used in a binding though. Do we care? (is it actually officially supported? => Edit: I guessyes 😃)

Also, a class might have an iterable typehint but expected injected value be an array. Shouldn't bindings support this too?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

It's supported since it always worked yes, and it works here also, this just skips the "must be a ref" check for scalar|array types.

a class might have an iterable typehint but expected injected value be an array

That's not supported right now. Could be added in another PR for sure (if worth it)

ogizanagi reacted with thumbs up emoji
@ogizanagi
Copy link
Contributor

I thinkRegisterControllerArgumentLocatorsPass misses an update to support typhinted bindings with scalars.

@nicolas-grekas
Copy link
MemberAuthor

@ogizanagi good catch, fixed thanks.

@nicolas-grekasnicolas-grekas merged commit32fc58d intosymfony:masterMay 21, 2018
nicolas-grekas added a commit that referenced this pull requestMay 21, 2018
This PR was merged into the 4.2-dev branch.Discussion----------[DI] Allow binding by type+name| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -This would allow to bind by type + argument name, e.g.:```yamlbind:  Psr\Log\LoggerInterface $logger:@logger```Allows more precise targets for bindings as it will match only if both the type and the name match.Works with scalar/array types also for consistency.Commits-------32fc58d [DI] Allow binding by type+name
@nicolas-grekasnicolas-grekas deleted the bind-tuple branchMay 21, 2018 18:18
nicolas-grekas added a commit that referenced this pull requestAug 24, 2018
…s-grekas)This PR was merged into the 4.2-dev branch.Discussion----------[DI] Allow autowiring by type + parameter name| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        |symfony/symfony-docs#10206In#27165, we introduced the possibility to bind by type+name:```yamlbind:    Psr\Log\LoggerInterface $myLogger: @monolog.logger.my_logger```But we forgot about aliases. For consistency, they could and should allow doing the same. More importantly, this will open up interesting use cases where bundles could provide default values for typed+named arguments (using the new `ContainerBuilder::registerAliasForArgument()` method). E.g:```yamlservices:    Psr\Cache\CacheItemPoolInterface $appCacheForecast: @app.cache.forecast```Works also for controller actions and service subscribers (using the real service id as the key).Commits-------c0b8f53 [DI] Allow autowiring by type + parameter name
@nicolas-grekasnicolas-grekas modified the milestones:next,4.1Nov 1, 2018
This was referencedNov 3, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

2 more reviewers

@ro0NLro0NLro0NL left review comments

@ogizanagiogizanagiogizanagi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.1

Development

Successfully merging this pull request may close these issues.

4 participants

@nicolas-grekas@ogizanagi@ro0NL@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp