add support for custom template file default suffix

[pine3ree]

PR for: zendframework/zend-expressive#641

I wonder if we should use the same configuration key in all compatible implementations (platesrenderer already support custom suffix with the extension config key)

[Xerkus]

I do not like much that configuration for the resolver needs to be added to template renderer like that. It feels out of place here tbh. I have no other suggestions at this point however.

[pine3ree]

Hello @Xerkus

I do not like much that configuration for the resolver needs to be added to template renderer like that. It feels out of place here tbh. I have no other suggestions at this point however.

I added the config entry for completeness, but it can be omitted because the default template resolver's suffix is 'phtml'. If a config entry exists, that will be injected into the resolver. Maybe we could comment out that entry with a clarifying note.

kind regards

[Xerkus]

@pine3ree let me clarify:
It is not about your change specifically but why you had to do it like that. A code purism issue if you like, in a broader scope than that of your PR.

The fact that you had to modify template renderer to add functionality, the configuration of dependency, that is not related to template renderer responsibilities is a violation of Separation of Concern principle.

Dependency is internal detail as long as you do not need external input for it, which makes it conditionally acceptable. Unit testing usually is a first place where the problem is exposed, when you need to provide alternative implementation for testing purposes.
Dependency Injection is a technique that provides separation of responsibilities by moving setup and configuration of dependency to external location. Dependency Injection Container, or simply Container, is usually that location.

The proper solution here is to remove dependency setup from template renderer and do that in a factory. That would, however, constitute a Backward Compatibility break so it would need a major version release to happen.

Now why it was done like that in the first place? Convenience of use was overriding factor when this code was initially implemented, it needed to just work and we had no convenient way to provide and configure container factories. It was before zend-config-aggregator component to consume ConfigProviders, zend-component-installer component to auto-inject ConfigProviders and config adapters for various container implementations.

So my dislike of the situation is just that: a dislike. I considered other possible approaches and your offers the least impact on the future compatibility with the next major release. That makes it the best we could do right now.

[pine3ree]

Hello @Xerkus,
I agree with everything you wrote (about separation of concerns in code), except one thing, if I understood correctly.

The fact that you had to modify template renderer to add functionality, the configuration of dependency, that is not related to template renderer responsibilities is a violation of Separation of Concern principle.

If you are referring to zend-expressive ZendViewRenderer, in my opinion it is not just a template "renderer" but an implementation of a very simple interface acting as a bridge to a whole package. zend-view in this case....well... parts of it.

zend-expressive template renderer interface has 4 methods:

• 1 for rendering the template (this implies being able to resolving it by name w/o an expressive resolver interface)
• 2 related to template paths
• 1 for adding default params into a template

so the interface itself tells us that ii is not just a "pure renderer" interface

Internally zend-expressive-zendviewrenderer creates the zend-view (php-)renderer, the resolver, the view model and allows to add search paths...and more...so adding a configuration parameter in the constructor only exploits what it is already doing....i.e. ... quite a lot....no much separation of concerns...the separation of concerns is implemented in the package it refers to!
The alternative would be to add more interfaces (renderer, resolver, model) for different concerns to the template part of expressive, but that would make things maybe incompatible to other renderer packages it now supports.

kind regards,
maks

[Xerkus]

@pine3ree nothing in this repository provides interoperability contract. So, anything that goes beyond what is established by Zend\Expressive\Template\TemplateRendererInterface is this adapter specific, including resolver and its configurations. As such there is no need to keep everything wrapped in template renderer.

I am going to provide a patch soon-ish that would resolve my concerns for the next major. You can ping me in zendframework slack and we can discuss this in detail if you like.

[pine3ree]

@Xerkus,
sure, thanks!
kind regards