KnpLabs / KnpMenuBundle

Object Oriented menus for your Symfony project.
http://knplabs.com
MIT License
1.4k stars 203 forks source link

Provide autowiring for MenuProviderInterface #384

Closed soullivaneuh closed 6 years ago

soullivaneuh commented 6 years ago

Current error:

Cannot autowire service "AppBundle\Menu\SidebarMenuBuilder": argument "$menuProvider" of method "__construct()" references interface "Knp\Menu\Provider\MenuProviderInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "knp_menu.menu_provider.chain", "knp_menu.menu_provider.lazy", "knp_menu.menu_provider.builder_alias", "sonata.admin.menu.group_provider". Did you create a class that implements this interface?

Current workaround:

services:
    _defaults:
        autowire: true
        autoconfigure: true
        public: false

    Knp\Menu\Provider\MenuProviderInterface: '@knp_menu.menu_provider'
stof commented 6 years ago

so, you actually inject the MenuProvider inside one of your menu builder ? I never thought about such use case. I would be interested to learn more about it

soullivaneuh commented 6 years ago

I have to copy the profile menu on the sidebar menu for the mobile view:

// Copy the profile menu for mobile view.
foreach ($this->menuProvider->get('profile')->getChildren() as $profileChild) {
    $menu->addChild($profileChild->copy()->setAttribute('class', 'visible-xs-block'));
}

I'm not sure it's the best way to do but it's the only valid way I found so far.

stof commented 6 years ago

What I did for similar case was either to put both menus in the same builder class (and using private methods for the common logic), or injecting a specific menu builder object inside my other builder and calling its method on it.

soullivaneuh commented 6 years ago

Well, it's a solution indeed, but the sidebar builder is enough large like that! :-D

soullivaneuh commented 6 years ago

In any case, having an autowiring solution here can be a useful thing. Plus, the "generic" service already exists.

stof commented 6 years ago

well, injecting my own service into the other service works with autowiring :smile:

But anyway, I'm OK with adding such autowiring. Can you send a PR ?