In #3258714: Manage attachments cache, the user cache context was added to markerio_page_attachments(). This means every page now varies by user.
By the time the HtmlRenderer is done calling all page_attachments hooks, we now have a HtmlResponse object that has the user cache context. This means DynamicPageCacheSubscriber will refuse to cache the page, even though the page itself might have been perfectly cacheable otherwise.
Enable this module, see that every page reports "UNCACHEABLE (poor cacheability)" in the "X-Drupal-Dynamic-Cache" header.
It seems you're adding some drupal settings in markerio_page_attachments, but can't you use a #lazy_builder somewhere during the render process? Then it will get placeholdered and the page will become cacheable again by DynamicPageCacheSubscriber.
When it gets to HtmlResponseSubscriber, the HtmlResponseAttachmentsProcessor will process the placeholders, adding your attachments to the response. After that, your JavaScript should still get added, but only after DPC has finished doing its thing.
I'm not sure if this widget is intended to be used by anonymous users, but if not then the above should be more than enough to fix the issue.
Rethink the caching strategy
/
/
/
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
kristiaanvandeneyndeP.S.: I'm willing to explain more about caching if you need guidance.
Comment #3
moshe weitzman commentedI wrote a patch for datalayer module that might be a helpful model - https://www.drupal.org/project/datalayer/issues/3345817
Comment #4
kristiaanvandeneyndeThat's exactly the type of fix I had in mind, thanks @moshe weitzman!
Comment #5
tijsdeboeckThanks @kristiaanvandeneynde @moshe weitzman, we're planning to work on this module in the coming weeks, and will update the caching strategy.
Regarding anonymous users, it is perfectly plausible that you want enable this for anonymous users. I can imagine that you would want to enable this during a test phase, where not all testers would have an account. So it is something we need to take into account.
Comment #6
vensiresComment #7
jacerider commentedWe ran into this on a current LIVE site. We had permission set so only admins would see Markerio. After debugging why our pages were resulting in UNCACHEABLE, we nailed it down to this module. Just wanted to +1 on this being a pretty important issue to resolve.
Comment #10
macsim commentedHere is a proposed fix.
The root cause is that
hook_page_attachments()unconditionally adds ausercache context, which makes every page report UNCACHEABLE to Dynamic Page Cache.The fix moves all user-dependent logic into a lazy builder (
LazyBuilder::build()), registered as a service.hook_page_attachments()now only adds theconfig:markerio.settingscache tag. A newhook_page_bottom()installs the lazy builder placeholder with#create_placeholder = TRUE, so Dynamic Page Cache can cache the page normally and the placeholder is resolved per-user at render time.Changes:
markerio.module: replacehook_page_attachments()logic withhook_page_bottom()+ lazy buildermarkerio.services.yml: new file registering themarkerio.lazy_builderservicesrc/LazyBuilder.php: new class implementingTrustedCallbackInterface, containing all user/route-dependent attachment logictests/src/Unit/LazyBuilderTest.php: 8 unit tests covering all branches ofbuild()Comment #11
macsim commentedComment #12
macsim commentedNote:
TrustedCallbackInterfacewas introduced in Drupal 8.8.0, not 8.0.0, so this patch is technically not compatible with the full^8range declared inmarkerio.info.yml. In practice this should not be an issue since Drupal 8 and 9 are both EOL. Dropping those fromcore_version_requirementwould be the clean solution, but that is a separate topic.Comment #13
tijsdeboeckThanks for the MR, we've tested this, and will ship a new release soon!