zendframework / zend-expressive-zendviewrenderer

zend-view PhpRenderer integration for Expressive
BSD 3-Clause "New" or "Revised" License
11 stars 12 forks source link

Retrieve the PhpRenderer from the container if it’s there… #39

Closed gsteel closed 6 years ago

gsteel commented 7 years ago

If you want to attach filters to the rendering engine filter chain for example, it’s not possible to access it from ZendViewRenderer so the factory is altered to retrieve a PhpRenderer from the container if one exists

stefanotorresi commented 7 years ago

@gsteel doesn't the test pass even if you don't add the has assertions on old tests? You generally shouldn't need to change previous tests, unless they were plainly wrong.

gsteel commented 7 years ago

@stefanotorresi The previous tests don't pass unless the mocked container is told to expect a call to has(PhpRenderer::class)

stefanotorresi commented 7 years ago

oh, I see now that the mock is a Prophecy one, which I'm not terribly familiar with, tbh. If it was a PHPUnit mock, not configuring the has method would be equal to configuring it to return null, so previous tests wouldn't need to be changed. I guess it doesn't matter then. ;)

mwillbanks commented 6 years ago

@gsteel @xtreamwayz generally speaking, if we are going to leverage the container, we should be fetching the interface not a concrete class. In this case, it should default to Zend\View\Renderer\PhpRenderer however we should actually have an interface fetching this. From an extensibility perspective that would be the better way.

weierophinney commented 6 years ago

Thanks, @gsteel!

gsteel commented 6 years ago

Thanks to you @weierophinney! Got my ZF contribution cherry ;)