zend-navigation icon indicating copy to clipboard operation
zend-navigation copied to clipboard

Zend\Navigation does not work with Zend\Expressive

Open RalfEggert opened this issue 9 years ago • 36 comments

I want to use Zend\Navigation in a Zend\Expressive application. First of all, I installed it:

composer require zendframework/zend-navigation

Composer installed it and added the ConfigProvider to my /config/config.php file:

$configManager = new ConfigManager(
    [
        Zend\I18n\ConfigProvider::class,
        Zend\Form\ConfigProvider::class,
        Zend\InputFilter\ConfigProvider::class,
        Zend\Hydrator\ConfigProvider::class,
        Zend\Session\ConfigProvider::class,
        TravelloViewHelper\ConfigProvider::class,
        new PhpFileProvider($pattern),
    ],
    $cachedConfigFile
);

When I try to use the navigation view helper in a template, I get this error:

A plugin by the name "navigation" was not found in the plugin manager Zend\View\HelperPluginManager

So, I added the Zend\Navigation\View\ViewHelperManagerDelegatorFactory to my configuration:

return [
    'dependencies' => [
        'delegators' => [
            Zend\View\HelperPluginManager::class => [
                Zend\Navigation\View\ViewHelperManagerDelegatorFactory::class,
            ],
        ],
    ],
];

No I get this error:

Unable to resolve service "Application" to a factory; are you certain you provided it during configuration?

After some investigation the problem is located here:

https://github.com/zendframework/zend-navigation/blob/master/src/Service/AbstractNavigationFactory.php#L102

The AbstractNavigationFactory seems to have a hard dependency on the MVC application which makes it impossible to use Zend\Navigation with Zend\Expressive currently.

Are there any plans to solve this? What should be done to break up this hard dependency? I guess it would be quite complicated to run Zend\Navigation both with Zend\Mvc and Zend\Expressive, wouldn't it?

RalfEggert avatar Dec 28 '16 07:12 RalfEggert

I also found out, that Zend\Navigation still refers to Zend\Mvc\Router in two places:

  • https://github.com/zendframework/zend-navigation/blob/master/src/Service/AbstractNavigationFactory.php#L16
  • https://github.com/zendframework/zend-navigation/blob/master/src/Page/Mvc.php#L12

RalfEggert avatar Dec 28 '16 07:12 RalfEggert

I also found out, that Zend\Navigation still refers to Zend\Mvc\Router in two places:

Right. At the moment, the navigation component supports the old router of the mvc component and the new router component:

use Zend\Mvc\Router as MvcRouter;
use Zend\Router\RouteMatch;
use Zend\Router\RouteStackInterface;

But I think this is not the problem. Please have a look at Zend\Navigation\Service\AbstractNavigationFactory::preparePages():

$application = $container->get('Application');
$routeMatch  = $application->getMvcEvent()->getRouteMatch();
$router      = $application->getMvcEvent()->getRouter();
$request     = $application->getMvcEvent()->getRequest();

This matches the error message you posted. And this is the problem!

Are there any plans to solve this?

Sure! 😃

I guess it would be quite complicated to run Zend\Navigation both with Zend\Mvc and Zend\Expressive, wouldn't it?

zend-navigation must run without zend-mvc.

froschdesign avatar Dec 28 '16 09:12 froschdesign

zend-navigation must run without zend-mvc.

Yep.

Currently Zend\Navigation depends on the Zend\Mvc\Application to access the Zend\Mvc\MvcEvent to access the RouteMatch, Router and Request objects. And its Zend\Navigation\Page\Mvc depends on the Zend\Router. So MVC is quite depply integrated.

To split this up we might need some integration components for Zend\Mvc and Zend\Expressive. Similar to the Zend\Mvc\I18n and packages. Sounds like a lot of work...

RalfEggert avatar Dec 28 '16 10:12 RalfEggert

Currently Zend\Navigation depends on the Zend\Mvc\Application to access the Zend\Mvc\MvcEvent to access the RouteMatch, Router and Request objects.

Right, but only in the current factories!

And its Zend\Navigation\Page\Mvc depends on the Zend\Router. So MVC is quite depply integrated.

Yes and no! 😉 The Zend\Navigation\Page\Mvc depends on zend-router, not zend-mvc! And zend-router is optional for the entire navigation component.

froschdesign avatar Dec 28 '16 10:12 froschdesign

Currently Zend\Navigation depends on the Zend\Mvc\Application to access the Zend\Mvc\MvcEvent to access the RouteMatch, Router and Request objects.

Right, but only in the current factories!

Nope :) Have a look into Zend\Navigation\Page\Uri::isActive and Zend\Navigation\Page\Mvc::isActive. The whole "isActive-Strategy" currently depends on a existing RouteMatch or - in case of Page\Uri - the Request object (nope, not the Psr\Http\Message\ServerRequestInterface, it's Zend\Http\Request). And AFAIK both - RouteMatch and Request - is not available outside the middleware chain.

I really tried to find a better way but for now thats what works for me. At least with the Page\Mvc.

/**
 * @param ContainerInterface $container
 * @param array|\Zend\Config\Config $pages
 * @return null|array
 * @throws \Zend\Navigation\Exception\InvalidArgumentException
 */
protected function preparePages(ContainerInterface $container, $pages)
{
    /** @var RouterInterface $router */
    $router = $container->get(RouterInterface::class);
    // mah... it's ugly, i know
    $request = ServerRequestFactory::fromGlobals();
    $routeMatch = $router->match($request);

    // HTTP request is the only one that may be injected
    if (! $request instanceof Request) {
        $request = null;
    }

    return $this->injectComponents($pages, $routeMatch, $router, $request);
}

tobias-trozowski avatar Dec 31 '16 11:12 tobias-trozowski

Nope :)

Please read carefully, because the content of Ralf's message was different:

Currently Zend\Navigation depends on the Zend\Mvc\Application…

Zend\Mvc is only used in the factories! 😃 Without the current factories you can use the mvc page only with zend-router. No need for the mvc component.

The whole "isActive-Strategy" currently depends on a existing RouteMatch or - in case of Page\Uri - the Request object.

The same here: the mvc component is not needed.

And AFAIK both - RouteMatch and Request - is not available outside the middleware chain.

Right, but that was not the content of Ralf's message.


$routeMatch = $router->match($request);

This creates a Zend\Expressive\Router\RouteResult, but the original injectComponents methods needs an object of Zend\Router\RouteMatch?!

froschdesign avatar Jan 02 '17 10:01 froschdesign

And yep, the match method will create a Zend\Expressive\Router\RouteResult. I had to overwrite the injectComponents too.

Didn't expect that zend-expressive have own implementation for the routing. I supposed zend-expressive uses zend-router.

tobias-trozowski avatar Jan 02 '17 14:01 tobias-trozowski

Didn't expect that zend-expressive have own implementation for the routing. I supposed zend-expressive uses zend-router.

We developed an abstraction around routing, to allow usage of any routing library, and provide three implementations: Aura.Route, FastRoute, and zend-router; FastRoute is the recommended router!

Thanks for opening this issue; clearly, we need to see if we can make it a bit more generic going forward!

weierophinney avatar Jan 04 '17 19:01 weierophinney

When you plan to fix the bug? I need to understand whether or not to wait for the decision or to use something else for menu.

Sohorev avatar Feb 06 '17 08:02 Sohorev

@Sohorev

…fix the bug?

This is not a bug, because zend-navigation was never designed and developed to use different types of router. Now we have to implement this new way.

When? I will look into over the weekend.

froschdesign avatar Feb 07 '17 10:02 froschdesign

@froschdesign that will be really great!

RalfEggert avatar Feb 07 '17 16:02 RalfEggert

@RalfEggert and @weierophinney I have created a first simple test: https://github.com/froschdesign/zend-navigation/commit/02190acea22340fec7a12457badd8cc7e7aba026

I wanted an implementation for version 2 of zend-navigation. Therefore, the test includes only new files and no changes in existing files.

(Notice: at the moment there is no support for multiple navigations)

Usage

dependencies.global.php

'dependencies' => [
    'factories'  => [
        Zend\Navigation\Navigation::class => Zend\Navigation\Service\ExpressiveNavigationFactory::class,
    ],
    'delegators' => [
        Zend\View\HelperPluginManager::class => [
            Zend\Navigation\View\ViewHelperManagerDelegatorFactory::class,
        ],
    ],
],

middleware-pipeline.global.php

'dependencies' => [
    'factories' => [
        Zend\Navigation\Middleware\NavigationMiddleware::class => Zend\Navigation\Middleware\NavigationMiddlewareFactory::class,
    ],
],
'middleware_pipeline' => [
    'routing' => [
        'middleware' => [
            ApplicationFactory::ROUTING_MIDDLEWARE,
            Zend\Navigation\Middleware\NavigationMiddleware::class,
            ApplicationFactory::DISPATCH_MIDDLEWARE,
        ],
    ],
],

Add a configuration for the navigation itself:

navigation.global.php

return [
    'navigation' => [
        'default' => [
            [
                'label' => 'Home',
                'route' => 'home',
            ],
            [
                'label'  => 'Add album',
                'route'  => 'album',
                'params' => [
                    'action' => 'add',
                ],
            ],
            [
                'label'  => 'Edit album',
                'route'  => 'album',
                'params' => [
                    'action' => 'edit',
                ],
            ],
        ],
    ],
];

froschdesign avatar Feb 12 '17 10:02 froschdesign

This looks great. Having an additional ExpressivePage might make it easy to adopt this. It is really a pain that I cannot use Zend\Navigation properly with Zend\Expressive. This will definitely help.

In the long run we should make Zend\Navigation independent from the MVC router if possible although I understand that it will be a lot of work.

:+1: :+1: :+1:

RalfEggert avatar Feb 13 '17 20:02 RalfEggert

independent from the MVC router

Maybe this move can one time address performance issues that are turning people away from it (or at least I have seen in IRC complain, not sure exactly why, I suspect it deals with redundant router match). isActive recursively comparing whether every page is active feels very redundant. Can this dependency be inversed, where after dispatch event completes if navigation module is present, it is told what the current active page is?

If navigation is told what is active, instead of the other way around, have opportunity to do better tree traversing, instead of brute-force recursive check of all nodes. If Z\Navigation stores its page node references not purely based on navigation hierarchy, but is "indexed" but route property of the pages, that index can be taken advantage of to quickly mark what page is active with $navigation->setActive($routerPath).

alextech avatar Feb 13 '17 20:02 alextech

@froschdesign @weierophinney

Any news? I am really looking forward for this patch to let Zend\Navigation work with Zend\Expressive.

RalfEggert avatar Feb 20 '17 09:02 RalfEggert

@RalfEggert Last saturday I added the tests and some fixes for the ExpressivePage class. More tests are on the way.

The support for multiple navigations is still missing.

froschdesign avatar Feb 20 '17 09:02 froschdesign

@froschdesign thankyou for you patch When you plan add it to composer package?

Sohorev avatar Feb 28 '17 10:02 Sohorev

@froschdesign This is great - we also want to use the navigation with zend-expressive! Will this be basically integrated into zend-navigation?

mano87 avatar Mar 06 '17 19:03 mano87

News

Code updates

  • NavigationMiddleware and NavigationMiddlewareFactory can now handle multiple navigation containers
  • ExpressiveNavigationFactory can be used for a single navigation (has no dependencies to zend-servicemanager)
  • ExpressiveNavigationAbstractServiceFactory can be used for a single or for multiple navigations (has dependencies to zend-servicemanager)

(not yet online - sorry!)

Next steps

  • complete all unit tests
  • creating a PR
  • calling for review

When

I hope tomorrow!

froschdesign avatar Mar 07 '17 22:03 froschdesign

Nice one! :+1:

RalfEggert avatar Mar 08 '17 05:03 RalfEggert

It just comes in time with Zend\Expressive

RalfEggert avatar Mar 08 '17 05:03 RalfEggert

@unnamed666 Please open a new issue with your question, instead of commenting on an unrelated one.

Alternately, use our new forums: https://discourse.zendframework.com.

weierophinney avatar Apr 27 '17 13:04 weierophinney

I wanted to create a new project based on Expressive and had the same errors as Ralf and saw that there is a pull request, which is still open. Could you tell us, when it will be possible to use zend-navigation in Expressive or what I could do to use it with the current version (if possible)? In the documentation of Expressive the navigation isn't mentioned.

av3 avatar Jun 13 '17 23:06 av3

@av3

In the documentation of Expressive the navigation isn't mentioned.

Right, because this feature is still work in progress.

You can use this branch for testing: https://github.com/froschdesign/zend-navigation/tree/feature/expressive-support

After testing, please come back and give us a feedback. Thanks!

froschdesign avatar Jun 14 '17 05:06 froschdesign

@froschdesign Thank you for your fast reply. I configured the files described. When using a single navigation, the output is working fine and the active menu item is getting his "active" class.

But I wasn't able to use a second navigation, even after adding the abstract_factory to the dependencies. I'm getting the error message

Unable to resolve service "Application" to a factory

Is there something wrong with my config or is the support of multiple containers missing in that branch?

av3 avatar Jun 14 '17 15:06 av3

@av3

Is there something wrong with my config…

I see nothing! 😉

…is the support of multiple containers missing in that branch?

No. (ExpressiveNavigationAbstractServiceFactory) Please compare your config with the example in the related PR.

froschdesign avatar Jun 14 '17 20:06 froschdesign

@froschdesign

Please compare your config with the example

First I used the config for the single container and with the 'default' block in my navigation config. In the second step I extended my dependencies.global.php and added a second block in my navigation config.

I see nothing!

My (complete) dependencies.global.php

return [
    'dependencies' => [
        'abstract_factories' => [
            Zend\Navigation\Service\ExpressiveNavigationAbstractServiceFactory::class,
        ],
        'aliases'            => [
            'Zend\Expressive\Delegate\DefaultDelegate' => Zend\Expressive\Delegate\NotFoundDelegate::class,
        ],
        'delegators'         => [
            Zend\View\HelperPluginManager::class => [
                Zend\Navigation\View\ViewHelperManagerDelegatorFactory::class,
            ],
        ],
        'invokables'         => [
            Zend\Expressive\Helper\ServerUrlHelper::class => Zend\Expressive\Helper\ServerUrlHelper::class,
            Zend\Expressive\Router\RouterInterface::class => Zend\Expressive\Router\ZendRouter::class,
        ],
        'factories'          => [
            'doctrine.entity_manager.orm_default'                     =>
                ContainerInteropDoctrine\EntityManagerFactory::class,
            Zend\Navigation\Navigation::class                         =>
                Zend\Navigation\Service\ExpressiveNavigationFactory::class,
            Zend\Expressive\Application::class                        =>
                Zend\Expressive\Container\ApplicationFactory::class,
            Zend\View\HelperPluginManager::class                      =>
                Zend\Expressive\ZendView\HelperPluginManagerFactory::class,
            Zend\Expressive\Template\TemplateRendererInterface::class =>
                Zend\Expressive\ZendView\ZendViewRendererFactory::class,
            Zend\Expressive\Delegate\NotFoundDelegate::class          =>
                Zend\Expressive\Container\NotFoundDelegateFactory::class,
            Zend\Expressive\Helper\ServerUrlMiddleware::class         =>
                Zend\Expressive\Helper\ServerUrlMiddlewareFactory::class,
            Zend\Expressive\Helper\UrlHelper::class                   =>
                Zend\Expressive\Helper\UrlHelperFactory::class,

            Zend\Stratigility\Middleware\ErrorHandler::class         =>
                Zend\Expressive\Container\ErrorHandlerFactory::class,
            Zend\Expressive\Middleware\ErrorResponseGenerator::class =>
                Zend\Expressive\Container\ErrorResponseGeneratorFactory::class,
            Zend\Expressive\Middleware\NotFoundHandler::class        =>
                Zend\Expressive\Container\NotFoundHandlerFactory::class,
        ],
    ],
];

My complete middleware-pipeline.global.php

return [
    'dependencies'        => [
        'factories' => [
            Zend\Navigation\Middleware\NavigationMiddleware::class =>
                Zend\Navigation\Middleware\NavigationMiddlewareFactory::class,
            Zend\Expressive\Helper\UrlHelperMiddleware::class      =>
                Zend\Expressive\Helper\UrlHelperMiddlewareFactory::class,
        ],
    ],
    'middleware_pipeline' => [
        'always'  => [
            'middleware' => [
                Zend\Expressive\Helper\ServerUrlMiddleware::class,
            ],
            'priority'   => 10000,
        ],
        'routing' => [
            'middleware' => [
                Zend\Expressive\Container\ApplicationFactory::ROUTING_MIDDLEWARE,
                Zend\Navigation\Middleware\NavigationMiddleware::class,
                Zend\Expressive\Helper\UrlHelperMiddleware::class,
                Zend\Expressive\Container\ApplicationFactory::DISPATCH_MIDDLEWARE,
            ],
            'priority'   => 1,
        ],
        'error'   => [
            'middleware' => [
                // Add error middleware here.
            ],
            'error'      => true,
            'priority'   => -10000,
        ],
    ],

My navigation block inside the config of my module:

    'navigation'   => [
        'default' => [
            [
                'label' => 'Startseite',
                'route' => 'home',
            ],
        ],
        'footer' => [
            [
                'label' => 'Impressum',
                'route' => 'imprint',
            ],
        ],
    ],

av3 avatar Jun 14 '17 21:06 av3

@av3 Do not register both navigation factories! Only ExpressiveNavigationAbstractServiceFactory.

froschdesign avatar Jun 15 '17 04:06 froschdesign

@froschdesign If I remove ExpressiveNavigationFactory (factories) and ViewHelperManagerDelegatorFactory (delegators), the error remains. And if I also remove the NavigationMiddlewareFactory (only from factories, the middleware entry remains), I'm getting the following error message:

Too few arguments to function Zend\Navigation\Middleware\NavigationMiddleware::__construct(), 0 passed in /application/vendor/zendframework/zend-expressive/src/MarshalMiddlewareTrait.php on line 145 and exactly 1 expected in /application/vendor/zendframework/zend-navigation/src/Middleware/NavigationMiddleware.php on line 32

av3 avatar Jun 15 '17 10:06 av3

@av3

I remove … ViewHelperManagerDelegatorFactory NavigationMiddlewareFactory

Why? Don't do this! For multiple navigations you need the factory ExpressiveNavigationAbstractServiceFactory and not the ExpressiveNavigationFactory - this is the only difference in the config between single and multiple containers.

froschdesign avatar Jun 16 '17 08:06 froschdesign