[DIC] Static injection #10991
[DIC] Static injection #10991
Conversation
|
Symfony PR added. |
|
Do you think it would be useful to add another wither method (eg, |
|
Actually, we can demonstrate both, IMHO, if we showcase both, we need to add a new service (maybe like the one in the PR?) in order to ease the readability. |
|
Here is a page that describes something similar, could be useful for inspiration and wordings. |
|
Maybe using things like |
There was a problem hiding this comment.
Thanks for working on this. Here as some comments to help (hopefully).
About the +/- balance, we should borrow from existing texts,eg
This time the advantages are:
- Immutable-setter injection works well with optional dependencies. If you do not
need the dependency, then do not call the setter. - You can call the setter multiple times. This is particularly useful if
the method adds the dependency to a collection. You can then have a variable
number of dependencies. - It works well with traits allowing one to compose features more easily. (to be added to the advantages of regular setters also)
- Like constructor injection, it allows services to remain immutable, by preventing the dependency to be changed during the lifetime of the object
The disadvantages of setter injection are:
- You cannot be sure the setter will be called and so you need to add checks
that any required dependencies are injected. - Unless the service is declared lazy, it is incompatible with services that reference each other in what are called circular loops
service_container/calls.rst
Outdated
| // ... | ||
| } | ||
|
|
||
| In order to use this approach fully, just adapt the container configuration: |
There was a problem hiding this comment.
We banned words like "just" recently. Here is an alternative proposal:
Because the method returns a separate cloned instance, configuring such a service means using the return value of the wither method ($service = $service->withLogger($logger);). The configuration to tell the container it should do so would be like:
There was a problem hiding this comment.
Improved, to check :)
| for the container to be capable of registering the method. | ||
|
|
||
| This approach is useful if you need to configure your service according to your needs, | ||
| so, what are the advantages? |
There was a problem hiding this comment.
I would remove the question and suggest a more direct sentence, eg:
The benefits of immutable-setters are:
There was a problem hiding this comment.
Improved, to be checked :)
| This approach is useful if you need to configure your service according to your needs, | ||
| so, what are the advantages? | ||
|
|
||
| * Your service becomes immutable, as the container will return a new object, |
There was a problem hiding this comment.
s/becomes/can remain/
the wither returns a new instance, but that's internal details, so not sure we need to mention it here
what matters is that consumer of the service cannot change its state
There was a problem hiding this comment.
Removed, to be checked 🙂
| the initial service stays clean and unchanged. | ||
|
|
||
| * You can easily change the injected service as long as it respect the interface/type | ||
| asked by the initial service. |
There was a problem hiding this comment.
I'm not sure what this means - isn't this true for dependency injection in general?
There was a problem hiding this comment.
Improved, to check :)
| * As the ``@return static`` docblock is required by the container to | ||
| understand that the method return a new object, | ||
| you can found that adding docblock for a single method isn't adapted or | ||
| link your code to the container. |
There was a problem hiding this comment.
not correct, so can be removed - the annotation is not required
…mutable services (nicolas-grekas) This PR was merged into the 4.3-dev branch. Discussion ---------- [DI] Add support for "wither" methods - for greater immutable services | 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#10991 Let's say we want to define an immutable service while still using traits for composing its optional features. A nice way to do so without hitting [the downsides of setters](https://symfony.com/doc/current/service_container/injection_types.html#setter-injection) is to use withers. Here would be an example: ```php class MyService { use LoggerAwareTrait; } trait LoggerAwareTrait { private $logger; /** * @required * @return static */ public function withLogger(LoggerInterface $logger) { $new = clone $this; $new->logger = $logger; return $new; } } $service = new MyService(); $service = $service->withLogger($logger); ``` As you can see, this nicely solves the setter issues. BUT how do you make the service container create such a service? Right now, you need to resort to complex gymnastic using the "factory" setting - manageable for only one wither, but definitely not when more are involved and not compatible with autowiring. So here we are: this PR allows configuring such services seamlessly. Using explicit configuration, it adds a 3rd parameter to method calls configuration: after the method name and its parameters, you can pass `true` and done, you just declared a wither: ```yaml services: MyService: calls: - [withLogger, ['@logger'], true] ``` In XML, you could use the new `returns-clone` attribute on the `<call>` tag. And when using autowiring, the code looks for the `@return static` annotation and turns the flag on if found. There is only one limitation: unlike services with regular setters, services with withers cannot be part of circular loops that involve calls to wither methods (unless they're declared lazy of course). Commits ------- f455d1bd97 [DI] Add support for "wither" methods - for greater immutable services
…mutable services (nicolas-grekas) This PR was merged into the 4.3-dev branch. Discussion ---------- [DI] Add support for "wither" methods - for greater immutable services | 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#10991 Let's say we want to define an immutable service while still using traits for composing its optional features. A nice way to do so without hitting [the downsides of setters](https://symfony.com/doc/current/service_container/injection_types.html#setter-injection) is to use withers. Here would be an example: ```php class MyService { use LoggerAwareTrait; } trait LoggerAwareTrait { private $logger; /** * @required * @return static */ public function withLogger(LoggerInterface $logger) { $new = clone $this; $new->logger = $logger; return $new; } } $service = new MyService(); $service = $service->withLogger($logger); ``` As you can see, this nicely solves the setter issues. BUT how do you make the service container create such a service? Right now, you need to resort to complex gymnastic using the "factory" setting - manageable for only one wither, but definitely not when more are involved and not compatible with autowiring. So here we are: this PR allows configuring such services seamlessly. Using explicit configuration, it adds a 3rd parameter to method calls configuration: after the method name and its parameters, you can pass `true` and done, you just declared a wither: ```yaml services: MyService: calls: - [withLogger, ['@logger'], true] ``` In XML, you could use the new `returns-clone` attribute on the `<call>` tag. And when using autowiring, the code looks for the `@return static` annotation and turns the flag on if found. There is only one limitation: unlike services with regular setters, services with withers cannot be part of circular loops that involve calls to wither methods (unless they're declared lazy of course). Commits ------- f455d1bd97 [DI] Add support for "wither" methods - for greater immutable services
…mutable services (nicolas-grekas) This PR was merged into the 4.3-dev branch. Discussion ---------- [DI] Add support for "wither" methods - for greater immutable services | 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#10991 Let's say we want to define an immutable service while still using traits for composing its optional features. A nice way to do so without hitting [the downsides of setters](https://symfony.com/doc/current/service_container/injection_types.html#setter-injection) is to use withers. Here would be an example: ```php class MyService { use LoggerAwareTrait; } trait LoggerAwareTrait { private $logger; /** * @required * @return static */ public function withLogger(LoggerInterface $logger) { $new = clone $this; $new->logger = $logger; return $new; } } $service = new MyService(); $service = $service->withLogger($logger); ``` As you can see, this nicely solves the setter issues. BUT how do you make the service container create such a service? Right now, you need to resort to complex gymnastic using the "factory" setting - manageable for only one wither, but definitely not when more are involved and not compatible with autowiring. So here we are: this PR allows configuring such services seamlessly. Using explicit configuration, it adds a 3rd parameter to method calls configuration: after the method name and its parameters, you can pass `true` and done, you just declared a wither: ```yaml services: MyService: calls: - [withLogger, ['@logger'], true] ``` In XML, you could use the new `returns-clone` attribute on the `<call>` tag. And when using autowiring, the code looks for the `@return static` annotation and turns the flag on if found. There is only one limitation: unlike services with regular setters, services with withers cannot be part of circular loops that involve calls to wither methods (unless they're declared lazy of course). Commits ------- f455d1b [DI] Add support for "wither" methods - for greater immutable services
8c81e56 to
af46b4f
Compare
|
@nicolas-grekas See this post this morning : https://symfony.com/blog/new-in-symfony-4-3-configuring-services-with-immutable-setters. Does the |
|
It's required anyway if you want to properly document your return types, isn't it? |
|
Is it WIP or not? I don't like hacking titles with tags... |
|
I've changed the title of this PR has the reviewed status has been added, sorry for the delay :/ |
service_container/calls.rst
Outdated
| The ``immutable-setter`` injection was introduced in Symfony 4.3. | ||
|
|
||
| When it comes to optional dependencies, the setter approach can be useful but | ||
| what if you need to change the injected dependency without mutating the initial service? |
There was a problem hiding this comment.
Alternative:
In order to provide immutable services, some classes implement immutable setters. Such setters return a new instance of the configured class instead of mutating the object they were called on::
| <!-- ... --> | ||
|
|
||
| <service id="app.newsletter_manager" class="App\Mail\NewsletterManager"> | ||
| <call method="withMailer" use-result="true"> |
| // ... | ||
| } | ||
|
|
||
| In order to use the full potential of this approach, you use a third argument |
There was a problem hiding this comment.
We didn't introduce setters yet, telling about the third argument is too early I think.
The sentence also is strange, the "full potential" is unclear.
Let's move this section after Setter Injection, and adjust (I'll review again after :) )
|
The feature was already merged in Thank you! |
5d4cc3b to
75162f8
Compare
|
Hi @OskarStark, Thanks for the reminder, I've pushed an updated version, if @nicolas-grekas has time to do a final review, I've rebased the PR. |
|
Thanks, can you please check DOCtor-RST and apply the warnings? Thank you 😊 |
This PR was merged into the 4.3 branch. Discussion ---------- [DIC] Static injection Hi everyone, As discussed with @nicolas-grekas, the DIC is planned to be capable of ensuring services immutability (using "wither" calls), the implementation can be found here: - symfony/symfony#30212 Commits ------- a598bc0 feat(DI): static injection
* 4.3: [#10991] Removed redundant 'in order to' feat(DI): static injection
* 4.4: [#10991] Removed redundant 'in order to' feat(DI): static injection
|
This took us waay to long to merge. Thanks a lot for keeping your PR up to date. During todays #SymfonyHackathon, this was one of my 3 must-merge PRs and I'm happy I made it. Hope you'll submit more PRs in the future :) |
Hi everyone,
As discussed with @nicolas-grekas, the DIC is planned to be capable of ensuring services immutability (using "wither" calls), the implementation can be found here: