Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This variable makes all templates require per page caching (or at least per (bool) $is_front_page
caching) in theory, even if it's never used.
Let's see if moving it to template_preprocess_page() breaks anything.
Comment | File | Size | Author |
---|---|---|---|
#14 | move_is_front_variable-2370147-13.patch | 2.96 KB | davidhernandez |
#14 | interdiff.txt | 1.37 KB | davidhernandez |
#10 | interdiff.txt | 471 bytes | lauriii |
#10 | move_is_front_variable-2370147-10.patch | 2.26 KB | lauriii |
Comments
Comment #1
Fabianx CreditAttribution: Fabianx commentedGood catch!
Comment #2
Fabianx CreditAttribution: Fabianx commentedRTBC, makes a lot of sense, we need to be more careful about this now.
Comment #3
jibranIt should be \Drupal::service('path.matcher')->isFrontPage() now.
Comment #4
catchComment #6
catchComment #7
jibranThank you for the fix.
Comment #8
alexpottWe need a change record for this.
Comment #9
almaudoh CreditAttribution: almaudoh commentedSmall nit...
Should be
\Drupal::service()
right?Comment #10
lauriiiComment #11
davidhernandezChange draft added. Am I correct in thinking that because this is in template_preprocess_page now instead of template_preprocess that the variable isn't accessible everywhere now, by default?
Comment #12
Fabianx CreditAttribution: Fabianx commentedThat is correct, but needed else every cache item is potentially 'PER_PAGE' suddenly ...
Comment #13
davidhernandezJust modifying some comments.
Otherwise, I'm fine with the change. I played with it some and it seems to work fine for page templates. On the negative side, this reduces the availability of the variable, but the performance improvement outweighs that concern. Any module/theme depending on the variable is easily fixed by checking for the front page directly.
While searching I noticed there are still a number of references to
drupal_is_front_page()
. Since that function is deprecated now, shouldn't those be changed? (out of scope for this issue) I didn't see an issue for it.Comment #14
davidhernandezForgot to upload the patches.
Comment #15
lauriiiLooks good to me
Comment #16
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 90a26b4 and pushed to 8.0.x. Thanks!
Comment #18
Fabianx CreditAttribution: Fabianx commentedYes, but as soon as they do that, they will need to ensure that the cache granularity of all objects on that page has the CACHE_PER_PAGE cache context.
Comment #19
davidhernandezFabianx, how do you do that? I want to add that information to the change record, but I'm not finding documentation for it or an implementation to use as an example.
Comment #20
Fabianx CreditAttribution: Fabianx commentedI honestly don't know how to do that.
I only know that core is setting #pre_render and #cache keys and that that contains cache contexts.
Wim: ^^
Nat: ^^
Comment #22
stefan.kornI am currently trying to figure out a good way to check for front page in a template other than page.html.twig.
Going to preprocess functions for each template that is needed?
I also tried to make a custom Twig Extension that checks with
\Drupal::service('path.matcher')->isFrontPage();
for the front page but this is not working, probably due to caching.Is there still an easy way to check for frontpage like in D7?
Would be great if someone can give me a hint.
Comment #23
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commented#22:
So whenever you check the front page you will also need to add a cache context so that the caching layer knows that your block or entity or whatever is encompassed is varied by 'route'.
However per route would be a lot of granularities.
Therefore dawehner proposed that in your contrib twig extension you also register a new cache context:
route.is_front
which just returns two states: TRUE if its the front-page or FALSE otherwise.
Your code in the extension would then need to add the cache context and the easiest is to just render a render array with that, like:
And thats it.
Comment #24
markhalliwell