Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autowiring via Service[] (WIP) #178

Merged
merged 1 commit into from Oct 23, 2018

Conversation

@dg
Copy link
Member

commented Oct 3, 2018

Autowiring of array of Service this way: (BC break)

class Foo
{
	public $services;

	/**
	 * @param Service[] $services
	 */
	public function __construct(array $services)
	{
		$this->services = $services;
	}
}
@f3l1x

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

So simple. 👍

@dg dg changed the title autowiring via Service[] (experimental) autowiring via Service[] (WIP) Oct 3, 2018

@dg dg force-pushed the dg:autowire-collection branch 2 times, most recently from 4700fe0 to a2cec9c Oct 3, 2018

@dg dg force-pushed the nette:master branch from 1312d1c to cf8afe5 Oct 3, 2018

@dg dg force-pushed the dg:autowire-collection branch 3 times, most recently from 73cff52 to 9144555 Oct 3, 2018

@milo

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

This is definetly time saver!

@milo

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

Without this (and previous tags() & types() commit), I'm solving it by extension. I have collections like:

class MyCollection
{
    public function add(MyInterface $service) {...}
}

and this DI extension which, in a short, finds all services of MyInterface type in container and creates MyCollection service with add() calls.

With proposed syntax, I don't need the extension anymore, because this collections have static factory createFromArray(array $items) too 👍

@enumag

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2018

I'm solving it with this package which is much more powerful. This only solves the most simple use-case so it's not nearly enough for me.

@dg

This comment has been minimized.

Copy link
Member Author

commented Oct 3, 2018

@enumag Can you describe what it does?

@enumag

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2018

@dg Sure. Basically when dealing with multiple services of the same type there are two use-cases:

  1. You need all of them because you need to "chain" them - run the same thing on all of them and collect the results. This is what you're solving here. My package does it too, but I'm using an Iterator instead of an array (that's a non-important detail). In my case the services need to be tagged and you can't simply autowire the iterator either so you definitely win here.

  2. Second use-case (and the reason why I wrote the package) is when you need some sort of a delegator to call specific implementation as needed. So the delegator only needs to call one or a few of the implementations, not all of them. Injecting an array is a no-go here because you want those services to be loaded lazily. So the package creates a callback (resolver) that can return a specific implementation based on some name. I've used this feature many times.

When integrating symfony/form to nette I also needed a third case which is in fact a combination of the first two - a resolver that returns an iterator of services. This was necessary for FormTypeExtensions - each FormType in symfony can have multiple FormTypeExtensions.


I realize it's not really too related to this PR, I just want to point out the fact that you rarely need all the services of the same type. The second use-case is much more common from my experience. You might want to consider addressing it too in the future.

@dg dg force-pushed the nette:master branch 2 times, most recently from 1fc7645 to 485a875 Oct 3, 2018

@dg

This comment has been minimized.

Copy link
Member Author

commented Oct 3, 2018

Point 2 looks useful, it is good idea to have multiple form of accessor.

interface DbAccessor
{
	function get(): Db; 
	// and allow this too:
	function get($name): Db
}

There should be enumeration of services in configuration, ie.:

services:
	- DbAccessor(
		name1: @a
		name2: @b
	)
	a: Db('***')
	b: Db('***')
	c: Db('***')
@enumag

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2018

Personally I prefer configuration with tags (the name is a tag attribute) instead of inventing some new syntax but that's a detail.

Which service would the function get(): Db; return? I don't see a reason to allow calling it without argument...

@dg

This comment has been minimized.

Copy link
Member Author

commented Oct 3, 2018

Currently there si supported this syntax

services:
	- DbAccessor(@a)

so it is only enhancement. Tags seems like another good way.

@enumag

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2018

Oh, right, forgot about that. Yeah this seems like a good improvement. 👍

@dg dg force-pushed the dg:autowire-collection branch from 9144555 to a96c6b4 Oct 3, 2018

@dg dg added this to the v3.0 milestone Oct 3, 2018

@dg dg force-pushed the nette:master branch from 700aca8 to c928e52 Oct 3, 2018

@dg dg force-pushed the dg:autowire-collection branch from a96c6b4 to 36162f3 Oct 4, 2018

@dg dg force-pushed the nette:master branch from 81a0e49 to ae3b10d Oct 4, 2018

@dg dg force-pushed the dg:autowire-collection branch from 36162f3 to d1c0598 Oct 4, 2018

@dg dg force-pushed the nette:master branch 2 times, most recently from a27a10b to 594a33a Oct 4, 2018

@TomasVotruba

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2018

I tried to implement this feature in Symfony, where it's hard to get into config yaml parsing or hack into autowiring.

https://github.com/rectorphp/rector/pull/660/files#diff-6303621e84b7d6786a9fd24b8c2be71d

Let's see how it works in practice :) thanks for inspiration

@dg dg force-pushed the dg:autowire-collection branch from d1c0598 to 0a386c0 Oct 6, 2018

@dg dg force-pushed the nette:master branch 2 times, most recently from 5f1f854 to 286f0d8 Oct 16, 2018

@dg dg force-pushed the dg:autowire-collection branch from af4a148 to e64586a Oct 16, 2018

@dg dg force-pushed the nette:master branch from 286f0d8 to e7fa17c Oct 17, 2018

@dg dg force-pushed the dg:autowire-collection branch from e64586a to 4341047 Oct 17, 2018

@dg dg force-pushed the nette:master branch from e7fa17c to 9298c49 Oct 23, 2018

@dg dg force-pushed the dg:autowire-collection branch from 4341047 to 855b4a7 Oct 23, 2018

@dg dg force-pushed the nette:master branch from 9298c49 to e18a432 Oct 23, 2018

@dg dg force-pushed the dg:autowire-collection branch from 855b4a7 to db22513 Oct 23, 2018

@dg dg force-pushed the nette:master branch from e18a432 to 9161f0c Oct 23, 2018

@dg dg force-pushed the dg:autowire-collection branch from db22513 to d70d7fd Oct 23, 2018

@dg dg force-pushed the nette:master branch from 9161f0c to ce6ae99 Oct 23, 2018

@dg dg force-pushed the dg:autowire-collection branch from d70d7fd to 0f952f2 Oct 23, 2018

@dg dg force-pushed the nette:master branch 2 times, most recently from a21dfbd to 66353fe Oct 23, 2018

@dg dg force-pushed the dg:autowire-collection branch from 0f952f2 to ff55601 Oct 23, 2018

@dg dg force-pushed the nette:master branch from 66353fe to 38a8a6e Oct 23, 2018

@dg dg force-pushed the dg:autowire-collection branch from ff55601 to 9a49252 Oct 23, 2018

@dg dg merged commit 1342085 into nette:master Oct 23, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@dg dg deleted the dg:autowire-collection branch Oct 23, 2018

dg added a commit that referenced this pull request Oct 23, 2018

dg added a commit that referenced this pull request Oct 23, 2018

dg added a commit that referenced this pull request Oct 24, 2018

dg added a commit that referenced this pull request Oct 24, 2018

dg added a commit that referenced this pull request Oct 25, 2018

dg added a commit that referenced this pull request Oct 25, 2018

dg added a commit that referenced this pull request Oct 25, 2018

@mabar mabar referenced this pull request Nov 30, 2018

Open

Acquisition: Arachne #2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.