LiipThemeBundle
LiipThemeBundle copied to clipboard
Add custom twig loader to workaround cache issue
Proof of concept solution for liip/LiipThemeBundle#79.
Apologies if I have missed something, but this seems as though it can be solved relatively simply by using a custom Twig filesystem loader, as stof suggested. This solution does not solve the 'referencing a non-themed template from a themed template' issue.
Suggestions are welcome.
fix #79
one of the unit tests needs to be fixed. also there are some minor CS issues (specifically missing space after if
and before (
@stof any feedback here?
also it would be good to have a functional test to illustrate that this works.
This won't work as is, because the class name for the compiled template still does not take the theme into account, so the Twig_Environment will not load the template a second time
Ah! Good point, I hadn't considered that. I'll have a look and see what changes would be necessary to solve the compiled class name issue.
@stof, I believe you are incorrect in your diagnosis. When determining the class cache name, Twig_Environment::getTemplateClass()
uses the return value from Twig_LoaderInterface::getCacheKey()
. The behaviour of the default Twig_Loader_Filesystem::getCacheKey()
is to simply use the value returned by findTemplate()
, which returns the full path to the file.
Twig_FilesystemLoader is an ancestor of our new loader so we have no problem, because findTemplate() returns the full path to the template, including the theme name (e.g. 'themes/themename').
I will fix the CS, the broken test, and create a new test for this feature.
@stof can you check once more?
alternatively @dantleech do you have time to look at this PR?
Would be great to add some tests for the compiler pass and the Loader (using prophecy would make this task easier).
Thanks for taking a look @dantleech! Lots of feedback there, I'll try to address it today and update this PR.
ping @xanido
anyone have time to wrap this up?
it wasn't fixed with #123 ?
@dantleech / @xanido can you get this wrapped up?
ping