search

EasyCorp / EasyAdminBundle

EasyAdmin is a fast, beautiful and modern admin generator for Symfony applications.
MIT License
3.99k stars 1.01k forks source link

Make the "ea" global variable dynamic #6273

Open nicolas-grekas opened 3 weeks ago

nicolas-grekas commented 3 weeks ago

My tentative fix for #5986

javiereguiluz commented 3 weeks ago

This could definitely be a solution that could work for us 🙏 Nicolas, thanks a lot for working on this. I'm going to test and look into this very soon!

You've probably seen it but ... in https://github.com/twigphp/Twig/issues/4007 some people are reporting that they face a similar issue but when using Shopware instead of EasyAdmin. I don't know if there's a way to solve this at Twig level for all projects, but I just wanted to mention it. Thanks!

nicolas-grekas commented 3 weeks ago

I tried relying on __call but that breaks the CI so I reverted to listing methods explicitly.

About shopware/etc. I suppose they do the same mistake. Possibly a RefreshGlobalsInterface could help. Or Twig could always refresh globals coming from extensions on reset. Anyway, that's not a short term solution for EA so not applicable.

A better option would be to deprecate the global and replace it with a function. This PR gives a hook to do so. This would have the benefit of freeing the hot path from loading anything related to EA, and I suppose that'd be the same for shopware.

My TL;DR is: don't use globals for non-static values.

stof commented 3 weeks ago

this will still break if templates do things like {% if ea is defined %}

nicolas-grekas commented 3 weeks ago

this will still break if templates do things like {% if ea is defined %}

True, and there is no way around this so we have to go with it IMHO.

Seb33300 commented 1 week ago

Is this PR also fixing this issue with another approach?

nicolas-grekas commented 1 week ago

Both should be merged, and a deprecation should be added here. Note that the getGlobals() method should be deprecated soon in twig.