mezzio / mezzio-laminasviewrenderer

laminas-view PhpRenderer integration for Mezzio
https://docs.mezzio.dev/mezzio/features/template/laminas-view/
BSD 3-Clause "New" or "Revised" License
6 stars 12 forks source link

ZendViewRendererFactory override HelperPluginManager #2

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

I have Factory which retrieves PhpRenderer from Container and injects this to my AssetMiddleware which prepare js/css files and appends them to view helpers (headLink, headStyle, inlineScript, headScript). During this time PhpRenderer initialize HelperPluginManager

public function getHelperPluginManager()
{
    if (null === $this->__helpers) {
        $this->setHelperPluginManager(new HelperPluginManager(new ServiceManager()));
    }
    return $this->__helpers;
}

But when I inject \Zend\Expressive\ZendView\ZendViewRenderer to my Action \Zend\Expressive\ZendView\ZendViewRendererFactory override HelperPluginManager which was initialized before

// Inject helpers
$this->injectHelpers($renderer, $container);

Factory simply check if HelperPluginManager was registered in Container. There is no check if PhpRenderer already has initialized HelperPluginManager

private function retrieveHelperManager(ContainerInterface $container) : HelperPluginManager
{
    if ($container->has(HelperPluginManager::class)) {
        return $container->get(HelperPluginManager::class);
    }

    if (! $container instanceof InteropContainerInterface) {
        throw new Exception\InvalidContainerException(sprintf(
            '%s expects a %s instance to its constructor; however, your service'
            . ' container is an instance of %s, which does not implement that'
            . ' interface. Consider switching to zend-servicemanager for your'
            . ' container implementation if you wish to use the zend-view renderer.',
            HelperPluginManager::class,
            InteropContainerInterface::class,
            get_class($container)
        ));
    }

    return new HelperPluginManager($container);
}

This behavior creates two different HelperPluginManager and creates many problems.


Originally posted by @popovserhii at https://github.com/zendframework/zend-expressive-zendviewrenderer/issues/54

weierophinney commented 4 years ago

Factory simply check if HelperPluginManager was registered in Container. There is no check if PhpRenderer already has initialized HelperPluginManager

If I understand you correctly, what you are writing here is correct behavior. The ZendViewRendererFactory does not check if the PhpRenderer has a HelperPluginManager registered.

I think what should be done in your custom PhpRendererFactory is grabbing the HelperPluginManager from the container and inject it into the renderer. This way your PhpRenderer and the ZendViewRenderer use the same HelperPluginManager.

class PhpRendererFactory
{
    public function __invoke(ContainerInterface $container) : PhpRenderer
    {
        $renderer = new PhpRenderer($config);

        if ($container->has(HelperPluginManager::class)) {
            $renderer->setHelperPluginManager(
                $container->get(HelperPluginManager::class)
            );
        }

        return $renderer;
    }
}

Originally posted by @geerteltink at https://github.com/zendframework/zend-expressive-zendviewrenderer/issues/54#issuecomment-364846974

weierophinney commented 4 years ago

Yes, I did it but this behavior isn't obviously. I waste 3 hours for debugging this issue. I check the hash of PhpRenderer in several places and it was identical. And I didn't understand why HelperPluginManager in my MiddlewareFactory is different to HelperPluginManager in the view template.

class MiddlewareFactory implements FactoryInterface
{
    public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
    {
        /** @var $asseticService AsseticService */
        $asseticService = $container->get('AsseticService');
        $renderer = ($container->has(PhpRenderer::class))
            ? $container->get(PhpRenderer::class)
            : new PhpRenderer();

        // this code was been added after debugging
        #if ($container->has(HelperPluginManager::class)) {
        #    $renderer->setHelperPluginManager($container->get(HelperPluginManager::class));
        #}

        return new AsseticMiddleware($asseticService, $renderer);
    }
}

If PhpRenderer has the possibility for default HelperPluginManager initialization, why I can't use this?

// vendor/zendframework/zend-view/src/Renderer/PhpRenderer.php:359
public function getHelperPluginManager()
{
    if (null === $this->__helpers) {
        $this->setHelperPluginManager(new HelperPluginManager(new ServiceManager()));
    }
    return $this->__helpers;
}

To my mind developer should not know internal logic. If he gets PhpRenderer from Container it should come with correct injected dependencies.


Originally posted by @popovserhii at https://github.com/zendframework/zend-expressive-zendviewrenderer/issues/54#issuecomment-364877462

weierophinney commented 4 years ago

Your MiddlewareFactory is good. What you need is a PhpRendererFactory, as I described above. In that PhpRendererFactory you require the HelperPluginManager the same way as the ZendViewRendererFactory to make sure you have the same instance of the HelperPluginManager.

To my mind developer should not know internal logic. If he gets PhpRenderer from Container it should come with correct injected dependencies.

In general you are right. However there is no PhpRendererFactory so this is something you need to create yourself. It's more of an edge case as not many developers use it this way. But I can see your point. I don't use the zend-view renderer myself so others need to decide if this a useful addition. You can always create a PR and add the missing PhpRendererFactory.


Originally posted by @geerteltink at https://github.com/zendframework/zend-expressive-zendviewrenderer/issues/54#issuecomment-364900817

weierophinney commented 4 years ago
  1. Yes, you are right there isn't any PhpRendererFactory and I simply register it as invokables

    'dependencies' => [
    'invokables' => [
        PhpRenderer::class => PhpRenderer::class,
    ],
    ]
  2. You can always create a PR and add the missing PhpRendererFactory

I clearly understood you, you suggest me create PhpRendererFactory and pull to a repository. Which one?

  1. ZendViewRendererFactory already check if HelperPluginManager was initialized in Container, it would be good check if it was already injected to PhpRenderer and doesn't override existed.

Originally posted by @popovserhii at https://github.com/zendframework/zend-expressive-zendviewrenderer/issues/54#issuecomment-364933605