laminas / laminas-view

Flexible view layer supporting and providing multiple view layers, helpers, and more
https://docs.laminas.dev/laminas-view/
BSD 3-Clause "New" or "Revised" License
74 stars 46 forks source link

A new assertion added into Layout prohibits use of OSS renderers such as provided by Twig. Would like to soften it. #172

Closed Saeven closed 2 years ago

Saeven commented 2 years ago

Feature Request

Q A
New Feature no
RFC no
BC Break somewhat

Summary

An assertion was added to Layout::getViewModelHelper() which prohibits OSS implementations of RendererInterface in the wild, for example, Twig's TwigRenderer.

protected function getViewModelHelper()
{
    if (! $this->viewModelHelper) {
        /**
         * @psalm-suppress DeprecatedMethod
         */
        $renderer = $this->getView();
        assert($renderer instanceof PhpRenderer); // offending assertion
        $helper = $renderer->plugin('view_model');
        assert($helper instanceof ViewModel);
        $this->viewModelHelper = $helper;
    }

    return $this->viewModelHelper;
}

Adding plugin() to RendererInterface seems a bit heavy handed, but relaxing the assertion to an interface would seem to be more logical, specially since the assertion only serves to protect the contract for ->plugin

Proposal

Would you be open to accepting a PR that either proposes to substitute this assertion for one that looks for an interface that provides plugin() (couldn't find an existing one), or one that adds plugin to RendererInterface (implemented by PhpRenderer)?

Ocramius commented 2 years ago

That code is hacky as hell.

Let's first remove the assertion, but we should really just get rid of such janky consteuct 😬

Saeven commented 2 years ago

Hey Marco, yeah I stopped short of using janky, but it's how it felt!

From a design contract point of view, I can appreciate wanting to protect the implementation - but none of it really fits. Protecting an implementation rather than its contract (non-existent in this case) seemed a bit sketchy.

I'm ready to pitch in - would you like

  1. a PR that simply proposes the removal, or
  2. a PR that adds plugin to RendererInterface (aggressive, but might be the right target?), and asserts on RendererInterface, or
  3. a PR that adds a new PluginInterface, implemented by PhpRenderer, and the assertion is on PluginInterface?

Hope you're doing well!

Cheers. Alex

Ocramius commented 2 years ago

Let's start from the first one 👍