Retrieve dependencies by FQCN where appropriate #193
Conversation
src/ConfigProvider.php
Outdated
| 'aliases' => [ | ||
| Annotation\AnnotationBuilder::class => 'FormAnnotationBuilder', | ||
| Annotation\AttributeBuilder::class => 'FormAttributeBuilder', | ||
| FormElementManager::class => 'FormElementManager', | ||
| 'FormAnnotationBuilder' => Annotation\AnnotationBuilder::class, | ||
| 'FormAttributeBuilder' => Annotation\AttributeBuilder::class, | ||
| 'FormElementManager' => FormElementManager::class, | ||
| ], | ||
| 'factories' => [ | ||
| 'FormAnnotationBuilder' => Annotation\BuilderAbstractFactory::class, | ||
| 'FormAttributeBuilder' => Annotation\BuilderAbstractFactory::class, | ||
| 'FormElementManager' => FormElementManagerFactory::class, | ||
| Annotation\AnnotationBuilder::class => Annotation\BuilderAbstractFactory::class, | ||
| Annotation\AttributeBuilder::class => Annotation\BuilderAbstractFactory::class, | ||
| FormElementManager::class => FormElementManagerFactory::class, |
There was a problem hiding this comment.
Note here: swapping aliases and factories has a secondary effect of messing with delegators.
I am not 100% sure if delegators operate the same way on factories or aliases. Consider keeping the priority as it was before :)
There was a problem hiding this comment.
I asked him to do so: I'd rather check the delegator issue than keeping this forever
There was a problem hiding this comment.
Reported in laminas/laminas-servicemanager#158
I hope to have a PR there in the next week
There was a problem hiding this comment.
This is still a BC break then: IMO not worth it for now, or to be extracted and targeted (just this bit) for 4.0.0
There was a problem hiding this comment.
It's a BC break only if the current laminas-servicemanager major doesn't work with both alias and aliased name
There was a problem hiding this comment.
If you manage to fix it, we can bump laminas-servicemanager to the fix version from laminas/laminas-servicemanager#158 and do it in a minor release.
Removing milestone here meanwhile, since it will be blocked for another while.
There was a problem hiding this comment.
Why don't I swap the aliases back for now - the rest can be merged preventing future conflicts?
IIRC, delegators simply don't work on aliases and never will: laminas/laminas-servicemanager#91
There was a problem hiding this comment.
Yeah, I think that would be OK to do meanwhile 👍
There was a problem hiding this comment.
@Slamdunk your call here - can also discuss it tonight.
gsteel
force-pushed
the
use-fqcn-for-container-retrieval
branch
from
f90f0ea to
b3eeadf
Compare
This patch updates all `$container->get()` call sites to use a FQCN instead of a string alias. DI configuration is updated so that factories are registered against FQCN but maintains existing service aliases. Signed-off-by: George Steel <[email protected]>
…erefore it's a BC break Signed-off-by: George Steel <[email protected]>
gsteel
force-pushed
the
use-fqcn-for-container-retrieval
branch
from
b3eeadf to
1d21cdb
Compare
|
Thanks @gsteel! |
Description
This patch updates all
$container->get()call sites to use a FQCN instead of a string alias. DI configuration is updated so that factories are registered against FQCN but maintains existing service aliases.