- ๐ช๐ธSpain pcambra Asturies
Just an aside comment, this patch wasn't working for me OOTB, and what I had to do is in a custom block extending the
LanguageBlock
one.Rather than using the render block, use the
getCacheContexts
method:/** * {@inheritdoc} */ public function getCacheContexts() { return Cache::mergeContexts(parent::getCacheContexts(), [ 'user.permissions', 'url.path', 'url.query_args', 'languages:language_content', 'languages:language_interface', ]); }
And rather than removing the
getCacheMaxAge
method, override it by:/** * {@inheritdoc} */ public function getCacheMaxAge() { return $this->cacheMaxAge; }
You need to include
use CacheableDependencyTrait;
in your custom block. - last update
over 1 year ago Composer error. Unable to continue. - last update
over 1 year ago 30,335 pass, 1 fail - Status changed to Active
4 months ago 10:49am 8 October 2024 - ๐ณ๐ฑNetherlands bbrala Netherlands
This is blocked on ๐ฑ Optimize render caching Needs work curretnly. But im not sure thats needed. Perhaps we can find another solution that allows us to fix this issue. That would unblock #2352009: [pp-3] Bubbling of elements' max-age to the page's headers and the page cache โ more, and i feel like the whole render cache issue is a bit to big and complicated.
- ๐ฎ๐ณIndia samit.310@gmail.com
samit.310@gmail.com โ made their first commit to this issueโs fork.
- Merge request !97982232375: Make language switcher block cacheable โ (Open) created by samit.310@gmail.com
- Status changed to Needs review
6 days ago 11:51pm 3 February 2025 - ๐จ๐ญSwitzerland berdir Switzerland
Many years later, there's actually progress here.
๐ Review cache bin and cache tags of access policy caching Active introduced a CacheoOptionalInterface, which is very close to what I've imagined back in 2016.
Implemented that and made BlockViewBuilder respect that.
The result of that is interesting.
--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php +++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php @@ -51,18 +51,18 @@ public function testFrontPageAuthenticatedWarmCache(): void { $expected = [ 'QueryCount' => 4, - 'CacheGetCount' => 40, + 'CacheGetCount' => 34, 'CacheGetCountByBin' => [ - 'config' => 22, - 'discovery' => 5, - 'data' => 7, + 'config' => 18, + 'discovery' => 4, + 'data' => 6, 'bootstrap' => 4, 'dynamic_page_cache' => 2, ], 'CacheSetCount' => 0, 'CacheDeleteCount' => 0, 'CacheTagChecksumCount' => 0, - 'CacheTagIsValidCount' => 10, + 'CacheTagIsValidCount' => 9, 'CacheTagInvalidationCount' => 0, 'CacheTagLookupQueryCount' => 5, 'ScriptCount' => 1,
On HEAD, LanguageBlock has a max-age 0 and gets placeholdered. That means on a dynamic page cache hit on umami, we need to load the block + language including translation overrides, block plugins and load routes. With this, the block isn't cached on its own, so we have one cache get less, but it's just part of the dynamic page cache. The downside of this is that we end up with url.query_args cache context in dynamic page cache. That didn't seem to cause any test fails so far, so we don't seem to have explicit test coverage for the cache contexts on dynamic page cache on a multilingual site with language switcher. Earlier attempts in this issue attempted to move thing into javascript. maybe we could just do that with the query string, unsure, or we placeholder just the query string or something like that.
- ๐ฌ๐งUnited Kingdom catch
placeholder just the query string
That sounds like a good thing to start with.
- ๐จ๐ญSwitzerland berdir Switzerland
It's easier said than done though. We have placeholders for single query string arguments, but this is different. Note that some language switch links can also add a langcode parameter, so it needs to deal with appending to that as well. maybe the whole URL could be a parameter for the placeholder? seems awkward.
It's also something that tends to be heavily customized/themed (icons, shortened languages, all kinds of completely custom designs), there's an alter hook and specific template suggestions for a link template. There's a fairly high chance that we'll break those customizations if we no longer pass in what we do now.
- ๐ฌ๐งUnited Kingdom catch
Oh I didn't realise it can have two query strings to individual placeholder and which also aren't guaranteed to exist, that does indeed sound painful.
We can set #create_placeholder explicitly on the whole block, per ๐ Create placeholders for more things Active . And then ๐ Render the navigation toolbar in a placeholder Active will mean it's no-longer rendered via big pipe when it's cached. That will keep the cache contexts isolated from the main dynamic_page_cache entry then but it'll still be render cached.
- ๐จ๐ญSwitzerland berdir Switzerland
Sounds good, I think it makes sense to wait on those two issues then.
Note: views also adds query_args as cache context to any view due to filters, sort and so on, realized that in the common cache tags issue with the new test. I was recently wondering on a project why dynamic page cache didn't kick in on a project where a bot (GPTBot) sent tens of thousands of requests that only varied in some tracking query string, that would be why). Might have some potential to limit dynamically based on having exposed filters/sorts/pager.
- ๐ฌ๐งUnited Kingdom catch
I don't think we need to wait on ๐ Create placeholders for more things Active , we can just copy the same pattern from that issue to here for the language switcher block. That issue has taken various twists and turns when I realised what it does and doesn't solve (completely different things to when I opened it), but the code changes are minimal/trivial. We don't necessarily need to wait for the cached placeholder strategy either although it will probably result in some test churn in either direction.
Seems like a good issue to open against views to add that conditionally.