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
Cleanup global $context
#2409
Comments
Hell yes, @nlemoine, great ideas! I pretty much agree with everything.
@jarednova might know more about this from the early days of Timber.
What would you put in there?
Handling requests probably falls in the same category as Routes. Because WordPress mostly works with rewrite rules and query vars by default, I think we shouldn’t be too opinionated about how to work with requests. I don’t know if this really needs to be in the context by default. One could argue that you probably shouldn’t work with request variables in Twig, because that’s more PHP land. But we live in an imperfect world and there are surely use cases where this is needed. Also, not everyone cares the same way about separation of concerns as some of us do :). In the end, Timber should make it easier to develop WordPress themes. I myself would rather have a nice way to work with requests in PHP. What if we made it optional as well with a list of recommendations (including http-foundation and diactoros) for how to extend Timber?
I agree, more lazyness is nice! But, what we then need to consider is a lot of developers work with dumps to see what’s in a variable. Lazyness hides a lot this information. So we definitely need good documentation about this. And maybe some way to show the information when dumping it (@acobster added some magic for that in https://github.com/timber/timber/blob/2.x/lib/PostArrayObject.php#L41). I can take care of the documentation part, if you like. |
Funny story, before opening this issue, I first wanted to submit a PR with some working code that moved But I faced some issues. $context = Timber::context(); // Where $context is a class that extends ArrayObject
Timber::render('template.html.twig', $context); Sadly, you can't pass an Using I'm not sure this can be resolved that way (but I'm very far from being the most skilled citizen in PHP land). I guess that's why Symfony uses an {# Will call getUser() #}
{{ app.user }} But that would be a breaking change for Timber. You could not call By the way, I'm thinking of something, why don't Timber uses $context = Timber::context();
Timber::render('index.html.twig', $context); Would become: Timber::render('index.html.twig'); // posts, etc. would already be here \o/ I understand about adding another dependency or being too opinionated. Maybe the |
Augh, fake polymorphism strikes again. This is my least favorite thing about PHP. :(
Yeah, I don't see a good solution. Since this is fundamentally an issue at the type system level, we'd have to get pretty hacky to get anything like it to work.
I strongly prefer being explicit. Magic has its own costs that can easily outweigh the annoyance of having an extra line of boilerplate. And since you'd have to loop over the keys to set the globals anyway, I think it would also preclude the laziness you're proposing? All in all, I think the way we get better laziness given current constraints is by making |
I’d like to throw in older discussions about how to handle the context, because it might be worth to revisit how contexts should be handled in the future.
An object-based context has been mentioned quite a few times. I wonder whether that might solve any of the problems we have here? And what other possibilities it would bring? I don’t want to say that we should tackle a complete context overhaul now, but we might get some idea for future updates that we’d have to consider now.
I have to agree with @acobster here. |
I thought a little more about keeping
If you see areas for improvement, that’s great! I just learned that |
Just came across this whilst doing some performance optimisation. For me the addition of For me personally I never use the body_class function due to all the fluff that WordPress generates there. Would be great if we could get a filter on adding it to the context to just skip that ( even on the V1 branch). Also really not sure what the point of the |
Is your feature request related to a problem?
2.x might be a good opportunity to make some
springwinter cleaning in the global context provided by Timber.IMHO, some values are useless:
http_host
: I wonder what would be the use case for this, should probably be a part of a request object (https://www.php-fig.org/psr/psr-7/#32-psrhttpmessagerequestinterface)wp_title
: Title tags are handled by all themes since quite a time. I don't think this necessary to include it in the global context anymore.Some values should be optin/moved to the user scope:
body_class
: There's many ways to do this. Add it to the global context yourself by usingtimber/context
filter or use{{ fn('get_body_class') }}
. One of the reason/benefit of using Timber is that it better decouples PHP from HTML. I hate having to use a WordPress filter (body_class
for example) to add a simple CSS class to an element... IMHO, removing this better fits Timber philosophy.Some values are redundant:
Theme
instance lives in$context['site']->theme
and$context['theme']
Objects should be lazy loaded in case they're not used:
theme
user
site
Some values are missing:
query
(e.g.$wp_query
): very useful because it gives access to conditional tags. Instead of doing{{ fn('is_singular', 'my_custom_post_type') }}
, you could do{{ query.is_singular('my_custom_post_type') }}
which is more fluent, consistent and nicer to me.debug
: less important but sometime useful tooSome may be enhanced:
request
object is very basic, I don't know if you would be open to adding another dependency but some requests classes would implement stronger features (symfony/http-foundation or laminas/diactoros)Describe the solution you’d like
A lighter global context with lazy instantiated objects.
In Symfony, the global context is handled by an
AppVariable
class which accesses services on demand. Although it would be hard to mimic this in Timber, I think the idea behind could be applied.The text was updated successfully, but these errors were encountered: