[DI] Reintroduce !service_locator in favor of !iterator #23542
Conversation
|
I think the reason is that we prefer explicit, which means favouring implementing ServiceSubscriberInterface and the corresponding container.service_subscriber tag. |
|
Well i think you still could write specialized service locators by decorating one, you get from config. My\ServiceLocator: { !service_locator { 'k': '@ref' } }I find this easier to follow as current |
|
I'm not sure we should follow this path. But let's say yes, I think we should make ServiceLocator implement IteratorAggregate, vs RewindableGenerator implement PSR 11. We should also ensure that factory closures are dumped only once. We could also have all tags generate code using this enhanced ServiceLocator, thus make PhpDumper simpler, and deprecate RewindableGenerator and maybe also IteratorArgument, while keeping the same set of tags + the one you're pushing for. |
|
I dont wanna push.. :) just having thoughts if we got this right. I tend to think we should go with plain verbose config; "give me this, here" instead of event based approach.
As
For cosmetic purposes that seems fine indeed. |
this sentence makes me think there is some misunderstanding somewhere:
I revert this idea, asking for an iterator should give you an iterator, not dual PSR11/iterable object :) |
To me this is PHP config. I see it's charming to have this at the class level, but your allowed to violate it anyway (right?) using I will play with
But do we care? User ask for iterable, we give an iterable. User asks for PSR container, we give on. The user should not have to rely on the implementation we provide, but only care about public API he needs. But maybe im being too pragmatic here :) |
|
We care because people will notice, then do mistake of asking for an iterator and using it as psr11. That's a trap. On the implementation level, I think we forgot something: thanks to IGNORE_ON_INVALID, iterators are allowed to skip values. That looks like the technical justification for the current clear split between both. All in all, maybe your tag is worth it and we should continue considering it, but implementation wise, I'm getting more and more convinced that the current code is the correct one. |
And a sign of bad typehints, but indeed, it may be to flexible after all.
Hm good point! Note there's no such thing as I guess we can keep it separated, nevertheless you can keep use !iterator as a map/service locator, as it preserves keys. Thats whats made me think we could combine it into a one-size-fits-all solution. So, basically;
Although it may help core things, i really intended this as a (imo. priceless) end-user feature. Hope you can appreciate that. |
|
Ok. im closing. Current features work fine individually, there's no real reason to combine. Though i still think it's a sane/pragmatic thing to do. Also we already have !service_locator in disguise using container.service_locator tag. Thats fine i guess. |
Basically merges
Di\Arg\RewindableGeneratorandDi\ServiceLocatorinto one. Introducing an internal god object that we can leverage to satisfy various typehints (think array, iterable, ContainerInterface). All lazy by default (well not array, we'd calliterator_to_arraybeforehand).See #22200 (comment) for more info.
Thoughts?