-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Twig doesn't support alternative PHP servers like RoadRunner #4007
Comments
Symfony has the The underlying issue is having a global variable defined conditionally in EasyAdmin. Instead it could be injected as parameter when templates are rendered (in one place in |
Proxy object for the |
Twig is perfectly able to handle multiple request: it can render several templates with the same |
@GromNaN Twig allows EasyAdmin to do this. So this is a Twig issue as well. The current case with EasyAdmin can be resolved by introducing a workaround in it. But any other Twig extension can fall into the same issue. This is why I suggest solving the root cause instead of relying on specific extensions to follow some unwritten agreement of not doing something. |
@GromNaN While I agree that Symfony has some tools to react to new requests in the same app, Twig is abstract from frameworks and, from my point of view, should support resetting its environment between requests. So then the Symfony bundle will be able to call this public Twig API with no hacks, workarounds, or extensions rewriting and solve the root cause for all cases, not for EasyAdmin specifically. |
Resetting the environment is almost the same as reinstantiating the Note that even if you reset the Twig environment, if you process 2 requests in parallel (co-routines with Fibers), you can end with an incorrect state for both of them. |
@GromNaN Why do we need to reinstantiate the whole instance if we need to cleanup only global variables? Is it already allowed in the Symfony integration? Fibers is a completely different topic and I would not mix them here. |
I experience similar issues, when running shopware/shopware with FrankenPHP. The globals are not reset between requests, which can cause significant rendering problems in Shopware, since the globals are, among others used for URL creation, and marking of page types. |
The issue is using globals for request-local state. |
Maybe twig should deprecate GlobalsInterface? |
Should be fixed by #4286 which introduces a new |
And for Symfony users, see symfony/symfony#58198 |
…P requests (fabpot) This PR was merged into the 5.4 branch. Discussion ---------- [TwigBundle] Add support for resetting globals between HTTP requests | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes-ish | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | License | MIT See twigphp/Twig#4007 and twigphp/Twig#4286 Commits ------- 42eb115 [TwigBundle] Add support for resetting globals between HTTP requests
Alternative PHP servers like RoadRunner don't unload PHP processes and process multiple requests per process allowing to get a significant performance increase. Unfortunately, Twig is not fully compatible with such servers because it relies on stateful classes that store some data. The intention is good - to cache data during the request. However, if the PHP process is not unloaded between HTTP requests, the internal state becomes an issue. On the second and further requests, Twig gets data from the first one which easily can be broken and make the request fail.
From my investigation, I understood that the issue is mostly in the global variables. They're cached in the
\Twig\Environment::$resolvedGlobals
property. Some extensions like EasyAdmin may add global variables conditionally. For example, if the first request doesn't meet EasyAdmin's criteria, theea
global variable is not loaded and not added to$resolvedGlobals
. On the next request, Twig sees the non-null$resolvedGlobals
and doesn't try to reload global variables. So any subsequent request that tries to render EasyAdmin's stuff will fail because theea
variable is not loaded.Currently, to fix this issue, I'm using the following workaround in my project:
Define a custom Twig class:
Add a compiler pass to replace the Twig object with the custom one:
The custom class subscribes to Symfony's
kernel.terminate
event and cleans up globals cache. So on the next request, Twig properly reinitializes it with variables for the current request.And the last point - to allow the Environment's
$resolvedGlobals
field modification, I apply the following patch withcweagans/composer-patches
:For my project, this works well. But I would be more than happy to see native support to not apply custom workarounds. The fix doesn't look complex so I believe it can be implemented quickly with no breaking changes.
The text was updated successfully, but these errors were encountered: