New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Macros imported to parent template are not accessible in child templates #1924
Comments
This looks like a bug to me. In 1.x, macros are compiled with a In 2.x, macros are now compiled with a The macro call in a child template (escaping excluded) is: $this->getAttribute((isset($context["macro"]) ? $context["macro"] : null), "hello", array(0 => "Fabien"), "method"); But Twig_Template::getAttribute do not implement this new prefix: if (isset(self::$cache[$class]['methods'][$lcItem])) {
$method = (string) $item;
} elseif (isset(self::$cache[$class]['methods']['get'.$lcItem])) {
$method = 'get'.$item;
} elseif (isset(self::$cache[$class]['methods']['is'.$lcItem])) {
$method = 'is'.$item;
} elseif (isset(self::$cache[$class]['methods']['__call'])) {
$method = (string) $item;
$call = true;
} As calling the macro in the template who imports it is just: echo $context["macro"]->macro_hello("Fabien"); I wonder if we should update the |
The compiler uses |
As the context is passed to child templates, the macro is well defined. It is just not callable because |
Is there a philosophical reason to not support this type of inheritance, or is merely a technical one that could be solved with the right PR? |
Yep, renaming the prefix introduced this issue. What do you think @stof? |
@ninsuo the fact that macros imported in the parent were available in the child templates is a side effect of the old implementation, and it was never supported officially. Note that we are aware of the fact it does not work anymore. This is why we were not able to change this in 1.x. |
Hmm, yeah, I just read the expression parser, you are right. So the old implementation behavior was a cool bug :) |
I think we could "fix" this but this will require deep work. It looks weird to add even more work to the already most perf-killing method ( Maybe copying the blocks behavior (a MacroReference in the function parser (well at least, a reference somewhere), then inheritance via doDisplay...)? |
Not sure, I want to keep macros, but allow macros inheritance just like blocks are inherited. |
@ninsuo normal macro calls were already treated at compilation time (they are not compiled through getAttribute, even in 1.x) |
If anyone is interested, I made a gist for an extension allowing to dynamically autoload macro from any views : see link |
It also work inside macro and set tags |
Made some tests with this gist and it works perfectly. Autoload works from absolutely everywhere without |
I'm going to close this one as the 1.x "feature" was actually a nice side effect (but it also came with even more not so nice side effects). With #3012, we are redefining the macro scope rules. |
Testing a Symfony 3/Twig 2 migration on a very large project and came across this.
Suppose we have in
parent.html.twig
:And then in another template:
This works in Twig 1.x but does not work in Twig 2 (on master). I can't find a specific reference to removing this in the documentation, and wonder if it is intended, since I can find no workaround other than importing commonly used macros into every template. On a large project this is very cumbersome, so I am hoping this is not intended.
If it is intended, is there a workaround for something like this or similar to this?
The text was updated successfully, but these errors were encountered: