winter: Can't get twig filter in macros

Winter CMS Build

1.2

winter/storm                        dev-wip/1.2 91993e2
winter/wn-backend-module            dev-wip/1.2 0258f85
winter/wn-cms-module                dev-wip/1.2 a998ed7
winter/wn-system-module             dev-wip/1.2 c5295ca
twig/twig                           v3.4.1

PHP Version

8.0

Database engine

MySQL/MariaDB

Plugins installed

No response

Issue description

Getting an error when using |page twig filter in a macro.

An exception has been thrown during the rendering of a template ("Undefined array key "this"").

Steps to replicate

composer create-project wintercms/winter winter12 "dev-wip/1.2 as 1.2"
php artisan winter:install
php artisan winter:env
php artisan winter:up

Then add {{ 'plugins' | page }} in default layout or home page.
Getting link to plugins page is ok.

If the same filter is call from within a macro, it fails.
Add a macro to layout and calling it :

{% macro test() %}
    {{ 'plugins' | page }}
{% endmacro test %}
{% import _self as home %}

{{ home.test() }}

Fall in an error.

Workaround

No response

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 18 (18 by maintainers)

Commits related to this issue

Add unit test for #578 — committed to wintercms/winter by bennothommo 2 years ago
Fix issues with accessing the expected context variables inside of Twig macros. This fixes #578 by adding the ability to pass the CMS Controller instance to the CMS Twig Extension removing the relian... — committed to wintercms/winter by LukeTowers 2 years ago

Most upvoted comments

I made some investigations: This is probably due to the PR https://github.com/wintercms/winter/pull/455 by @LukeTowers. As I understand, in this PR, he dissociated the Twig base environment from the cms extension.

Doing so caused the CMS environment to not “overlaps” the Twig’s one, making the this context key not available anymore from the original Twig’s MacroNode context here.

I made a dirty test to convince myself, replacing the twig package line linked previously by this one:

->write("\$context = \$this->env->mergeGlobals(array_merge(['this' => 'test'],\n")
// And another tiny-modification after that just to close the parenthesis

Doing so make the this key found (of course another error is raised because the controller key can’t be found in the this array).

I looked into the Twig documentation and found how to add a global variable. I was able to fix this issue by modifying the initTwigEnvironment method from the cms Controller class:

protected function initTwigEnvironment()
{
    $this->twig = App::make('twig.environment.cms');
    $this->twig->addGlobal('this', ['controller' => $this]);
}

Unfortunately, I am not sure if this is the most efficient way to do, and if some other global variable should be injected to prevent other related issue?

RomainMazB on Jun 24, 2022

Well, looks like we have a winner!

This resolve the issue with the DynamicPDF plugin as well.

mjauvin on Jul 10, 2022

Good find @RomainMazB !

Whether or not the global var is the right fix, at least we have a good pointer on where the problem lies.

mjauvin on Jun 24, 2022

I’m happy for you guys to proceed with a PR. Might have a couple of comments then, but will be better to test things in action.

bennothommo on Jun 30, 2022

I’ll wait for Ben’s feedback before working on this. I could submit a PR including tests this week.

RomainMazB on Jun 27, 2022