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 "instanceof" section for local interface-defined configs#21530

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
fabpot merged 2 commits intosymfony:masterfromnicolas-grekas:instanceof
Feb 17, 2017

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedFeb 4, 2017
edited
Loading

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

This is a direction follow up of#21357 on which we're working together with@dunglas. From the description posted there:

There is some work being done to include features ofDunglasActionBundle in the core of Symfony. The goal of all those PRs is to improve the developper experience of the framework, allow to develop faster while preserving all benefits of using Symfony (strictness, modularity, extensibility...) and make it easier to learn for newcomers.

This PR implements the tagging feature of ActionBundle in a more generic way. It will help to get rid ofAppBundle in the the standard edition and to register automatically some classes including commands.

Here is an example of config (that can be embedded in the standard edition) to enable those features:

# config/services.ymlservices:_defaults:# Enable constructor and getter autowiring for all services defined in this fileautowire:['get*']public:false_instanceof:# Add the console.command tag to all services defined in this file having this typeSymfony\Component\Console\Command\Command:tags:['console.command']public:true# commands must be publicTwig_ExtensionInterface:tags:['twig.extension']Symfony\Component\EventDispatcher\EventSubscriberInterface:tags:['kernel.event_subscriber']# Register all classes in these directories as servicesApp\:resource:../src/{Action,Command,EventSubscriber,Twig}

It's part of our 0 config initiative: controllers and commands will be automatically registered as services and "autowired", allowing the user to create and inject new services without having to write a single line of YAML or XML.
When refactoring changes are also automatically updated and don't require to update config files. It's a big win for rapid application development and prototyping.

Of course, this is fully compatible with the actual way of defining services and it's possible to switch (or mix) approaches very easily. It's even possible to start prototyping using 0config features then switch to explicit services definitions when the project becomes mature.

theofidry, Taluu, fesor, grachevko, enleur, andrewmy, hason, janit, mutm, hhamon, and emirb reacted with thumbs up emojisstok, enleur, jdreesen, and hhamon reacted with heart emoji
@stof
Copy link
Member

stof commentedFeb 7, 2017

what happens when you use a parameter as class name for the service (which is still supported even though we discourage its usage in the doc) ?


/**
* @param string $parent The id of Definition instance to decorate
* @param string|null $parent The id of Definition instance to decorate
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the idea of allowing null here. A ChildDefinition registered in the container must not have a null parent id (it would break things).

If you use a ChildDefinition temporarily to resolve instanceof configurations in the loaders, I would rather use a dummy parent id (which will not go outside the loader anyway)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

reverted

@stof
Copy link
Member

stof commentedFeb 7, 2017

what happens when the service class matches multiple_instanceof ? and what happens for services defined using a factory ?

@nicolas-grekas
Copy link
MemberAuthor

what happens when you use a parameter as class name for the service?

the class is resolved using the parameterbag already.
of course that can't account for params added later in compiler passes.
in that case (ie when the class is missing really), nothing special happens (ie we continue as usual).
SeeResolveDefinitionTemplatesPass.php

what happens when the service class matches multiple _instanceof ?

they are applied on top of each others in declaration order

and what happens for services defined using a factory ?

nothing special: we use the "class" attribute, which we deprecated not defining, so we can use "instanceof" even in the case of factories.


/**
* @experimental in version 3.3
*/
Copy link
Member

Choose a reason for hiding this comment

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

we should document the type of the argument (what is inside the array ?)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

done


/**
* @experimental in version 3.3
*/
Copy link
Member

Choose a reason for hiding this comment

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

we should document the returned type (an array of something, but what ?)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

done

@nicolas-grekasnicolas-grekasforce-pushed theinstanceof branch 7 times, most recently from24d8139 to8f5a69dCompareFebruary 9, 2017 14:38
return$this;
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

Missing dot at the end.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

fixed

@nicolas-grekasnicolas-grekasforce-pushed theinstanceof branch 7 times, most recently from9f89907 toa06d87cCompareFebruary 13, 2017 15:56
@dunglas
Copy link
Member

👍

{
if ($this->isLoadingInstanceof) {
if (!$definitioninstanceof ChildDefinition) {
thrownewInvalidArgumentException(sprintf('Invalid type definition "%s": ChildDefinition expected, %s given.',$id,get_class($definition)));
Copy link
Member

Choose a reason for hiding this comment

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

missing" around the second%s

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

fixed

@fabpot
Copy link
Member

Thank you@nicolas-grekas.

@fabpotfabpot merged commit773eca7 intosymfony:masterFeb 17, 2017
fabpot added a commit that referenced this pull requestFeb 17, 2017
…al interface-defined configs (nicolas-grekas, dunglas)This PR was merged into the 3.3-dev branch.Discussion----------[DependencyInjection] Add "instanceof" section for local interface-defined configs| 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 is a direction follow up of#21357 on which we're working together with@dunglas. From the description posted there:There is some work being done to include features of [DunglasActionBundle](https://github.com/dunglas/DunglasActionBundle) in the core of Symfony. The goal of all those PRs is to improve the developper experience of the framework, allow to develop faster while preserving all benefits of using Symfony (strictness, modularity, extensibility...) and make it easier to learn for newcomers.This PR implements the tagging feature of ActionBundle in a more generic way. It will help to get rid of `AppBundle` in the the standard edition and to register automatically some classes including commands.Here is an example of config (that can be embedded in the standard edition) to enable those features:```yaml# config/services.ymlservices:    _defaults:        autowire: ['get*', 'set*'] # Enable constructor, getter and setter autowiring for all services defined in this file    _instanceof:        Symfony\Component\Console\Command: # Add the console.command tag to all services defined in this file having this type            tags: ['console.command']            # Set tags but also other settings like "public", "autowire" or "shared" here        Twig_ExtensionInterface:            tags: ['twig.extension']        Symfony\Component\EventDispatcher\EventSubscriberInterface:            tags: ['kernel.event_subscriber']    App\: # Register all classes in the src/Controller directory as services        psr4: ../src/{Controller,Command,Twig,EventSubscriber}```It's part of our 0 config initiative: controllers and commands will be automatically registered as services and "autowired", allowing the user to create and inject new services without having to write a single line of YAML or XML.When refactoring changes are also automatically updated and don't require to update config files. It's a big win for rapid application development and prototyping.Of course, this is fully compatible with the actual way of defining services and it's possible to switch (or mix) approaches very easily. It's even possible to start prototyping using 0config features then switch to explicit services definitions when the project becomes mature.Commits-------773eca7 [DependencyInjection] Tests + refacto for "instanceof" definitions2fb6019 [DependencyInjection] Add "instanceof" section for local interface-defined configs
@nicolas-grekasnicolas-grekas deleted the instanceof branchFebruary 17, 2017 22:04
@fabpotfabpot mentioned this pull requestMay 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot left review comments

@stofstofstof left review comments

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

5 participants

@nicolas-grekas@stof@dunglas@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp