-
Notifications
You must be signed in to change notification settings - Fork 196
Provide template factories and facilities #128
Provide template factories and facilities #128
Conversation
This patch imports the factories introduced at: - https://github.com/xtreamwayz/expressive-composer-installer/tree/master/src/App/Template for each of Twig, Plates, and ZendView, with modifications (particularly the lack of default template paths). Additionally, unit tests have been added to verify behavior of each container. The Twig support now includes the TwigExtension, which overrides the path and asset twig helpers in order to delegate to zend-expressive functionality.
- The Whoops runtime should now be injected with a pretty page handler, per zendframework#125.
Ping @xtreamwayz - still in progress, but I think this will make the work in the installer easier. :) |
thanks to @xtreamwayz and @weierophinney for this. We are looking forward to the zend-view integration especially set up of the We currently use three factories to set up zend-view: Whereby the |
@weierophinney I've made an installer test branch here: composer-installer/feature/template-factories I was already hoping you would add the template factories. Thanx a lot. Everything is working out of the box now. One issue tough: The default extension for twig should be .html.twig. This is not a big deal but it's better than .html. Mainly because IDE's pick up the .html.twig extension and the other reason is listed here: The template name also has two extensions that specify the format and engine for that template. Also I used zend-config for the configuration container. A class is needed to inject the config into Aura.Di. But zend-config fails on the is_array check. Fortunately the standard ArrayObject class does work, which will be used now. It also removes the zend-config dependency. @codeliner The url helper class and tests are there, but it's not injected yet. I guess you have to wait until the box in front of this is ticked: Push into helper manager via ZendViewFactory |
@xtreamwayz damn, of course I didn't read carefully enough. However, I'm excited about the solution. As noted above we had a hard time figuring out a way to get a custom view helper into zend-view when only working with a application wide But yeah, let's wait until the box is ticked. |
- Consumes PSR-7 request URI.
The zend-view container factory now also injects the HelperPluginManager instance, adding overrides for the url and serverurl helpers.
Fixed!
Is there an action item here? Or were you indicating that the approach I used solves an issue you had in your original implementation? The zend-view factory now will inject the custom view helpers into the helper plugin manager. |
@weierophinney I actually wanted to tell you that the is_array check gave errors. When writing it down I thought of a different way to get it working. So it was more an action for me. |
@weierophinney Thx! Works like a charm. I added an additional |
Sure; send a pull request. :-)
|
Hi, I may be completely wrong, but if I look at that: https://github.com/zendframework/zend-expressive/blob/master/src/Container/Template/TwigFactory.php#L49 it looks to me that you are basically expecting this to be in the main config array returned by "config". This seems a bit fragile to me. I mean, all the key names "debug", "templates" are super common and you could have a lot of weird bugs when everything will be merged at some point into the "config" service. I'd advocate that we should adopt a three levels structure for the container: "component" > "sub-component" > "option", so in this case the config expected would be: return [
'expressive' => [
'twig' => [
'debug' => // ...
]
]
] |
The debug setting is inherited from my installer. The reason behind it is that your app is in dedug mode or not. It makes sense to move it into the temple array though if there is a need to debug only your templates. Normally you would be right with a structure like that. But I don't think expressive is the component in your example. The template engine is the component. Expressive just provides the factories for your convenience. And template is just a common name for all the different engines supported by expressive. And so far I haven't found any weird bugs with this naming. I guess it might be an issue when you use 2 different template engines at the same time for whatever reason. |
Hi, Thank you for the clarification. It seems that indeed the "debug" is a global Zend Expressive config. I'd say that we need to have a clear way to structure config, and I like the component > sub-component > config way (this is what @mnapoli suggested too iirc). This would open the path to use something like Puli for merging config (which should be top priority at the moment in my opinion :D). By following this logic, it shoudl look like this: return [
'expressive' => [
'debug' => true,
],
'aura' => [
'router' => [
// Router config
]
],
'fast_route' => [
'router' => [
// Router config
]
],
'zf2' => [
'templates' => [
// ZF2 templates
],
'router' => [
// ZF2 router
]
],
'twig' => [
'templates' => [
// Twig templates
]
]
]; And so on... Not only this make thing much coherent, and encourage using the same style for third-party library (as well as providing a common structure that would work even when merging config from unknown third parties), but it would also allow to have templates both from ZF2 and Twig transparently, for instance. I find it a bit strange that templates are routes seems to be configured quite differently in Zend Expressive. What do you think @weierophinney ? |
👍 for @bakura10 suggestion. We successfully used interop-config which suggest the same way to structure component config. In our example application we used it to write a factory for Doctrine DBAL. But I think |
Definitely. What confuses me a bit is the fact that routes are actually configured as part of the Expressive application factory, while the templates have their own dedicated factory. So maybe we could use a unifided system as you suggested: return [
'expressive' => [ // Or application
'routes' => [
],
'templates' => [
]
]
]; Then, instead of having the routes configuration pulled here: https://github.com/zendframework/zend-expressive/blob/master/src/Container/ApplicationFactory.php#L152 We would instead have a dedicated factory for AuraRouter, Zf2Router and FastRouteRouter that would fetch them. This would make this more coherent with how templates work. |
@bakura10 good catch. 👍 for dedicated factories. |
There's a key difference between templates and routes however: Application Another difference is that for template paths, we can operate on the Routers are more likely to benefit from adapter specific configuration due I definitely see the point of standardizing the configuration; I've not
|
Good to know that. Based on the docs my assumption was that they are interchangeable. But yeah, only static routes will continue to work, dynamic ones require small to medium adjustments. |
I'm doing a little analysis here, and I'm a bit stuck, as we have a few things going on:
The Next, we have the Finally, we have templates. These differ from the other concerns in that:
Like routes, however, there is an abstraction-specific aspect: paths. My thought on this is to make them follow the routing paradigm for configuration: each will attempt to pull the templating engine from the container if possible, allowing for component-specific configuration if desired and using a default instance if no service is found. From there, paths will be injected into the template abstraction. One other aspect that differs with templates is that two of the three implementations have additional constructor arguments that are specific to their implementations of the abstraction (e.g., layout for ZendView). As such, they will also need expressive-specific configuration, in addition to potential component/engine-specific configuration. This would need to live under a "templates" or "template" sub-key of the expressive configuration. This leaves us with exactly one decision to make: where should the "routes" and "templates" configuration live? Do we provide a top-level "zend-expressive" key and push them under that? or keep them as top-level (as they are now)? Regardless, I'll start working on the changes to the template factories a noted above (allowing pulling the engine as a service). |
First of all, I'm writing this as a developer of the expressive installer. If you haven't seen it yet, please have a look. There are a few namespaces we are talking about here. They are:
Explanation:
As you can see, the only thing that I didn't mention is the container. This is basically outside the scope of zend-expressive. It expects a container-interop, but it depends on the user on how to load and set it up. Besides that, for the sake of simplicity of the installer, it has to be the same name for all different containers. Just have a look here so you can see how it relies on 1 namespace for the container between all related dependencies. Having said all this, if you need a proper namespace, I suggest something like this: return [
'dependencies or container' => [
],
'zend-expressive or expressive' => [
'middleware or middleware_pipeline' => [],
'routes' => [],
'templates' => [
'debug' => BOOLEAN // Just for twig
],
'whoops or error_handler' => [],
],
]; AS I said before, the container is out of the scope of expressive. So basically it's up to the installer or the user to name it. For the installer it has to be a unified name for simplicity. I don't know what the standard whoops config is, but if it is different, just place it inside the expressive namespace. Same goes for other config. If it is a config forced by expressive or its factories, add it to the expressive namespace. I know this sounds strange to zend-framework users since zend-framework has an excellent namespacing. However, this is not zend framework. As I understand, expressive is a middleware micro-framework that relies on diactoros and stratigility. Besides that, it makes a difference from other middleware frameworks by supporting multiple php packages. I'm just saying, keep it as simple as possible to attract a bigger audience. |
Finally took a look at interop-config, and it looks quite interesting, in part because it may simplify a ton of the logic in the factories themselves. Will investigate as I start refactoring; thanks, @codeliner ! @xtreamwayz — I agree with keeping it as simple as possible. However, I also think consistency of configuration is important (it's inconsistent currently; see my notes on error handling versus routes versus templates) and consistency of implementation is important (see my notes on routes versus templates). Having a consistent system makes learning it easier. While I'm not a huge fan of nesting the configuration deeper, it does provide those particular benefits. Let me try it out, and we'll see how it ends up. |
This patch aims to address #122 and #123, by providing container factories for each template engine, as well as useful facilities for integrating them with Expressive.
ServerUrl
Url
ZendViewFactory