[1.x][2.0.x] Better caching for cookie performance popup

We can't use render cache, but we might be able to just put everything into a single regular cache entry and fetch from that instead of having to build it over and over again on every page?

Why can't we use render cache? The three existing parts of the $cid (language ID, domain ID, theme name) are already covered by the render cache. In fact, having to add info from another module, the domain module, is a code smell that perhaps this isn't the best approach. The domain module's documentation suggests users add the url.site cache context so that the cache can vary based on the domain. So the EU Cookie Compliance module shouldn't need to worry about the domain. However, because it micromanages this part of the renderable content, it needs to account for all the ways it may vary.

In working on the 🐛 DNT (Do Not Track) header detection not working when caching enabled Active bug, I traced the issue to the custom cache entry's $cid not varying based on the DNT header. If a custom cache entry was not deployed, that bug would easily be fixed with one line of code adding the DNT header to the cache context. Instead, we need yet another variance of the $cid.

So to summarize, why can't we let the render cache do its job? Micromanaging a separate cache entry causes additional complexity, makes it easier to cause bugs (DNT header issue), and causes this module to care about unrelated modules (domain module).

Reopening this issue after comment from @pianomansam.

hi @pianomansam

How much of this change are we talking about undo'ing?

@atowl the merge request on 🐛 DNT (Do Not Track) header detection not working when caching enabled Active does what I propose (removing the custom cache entry). The only line of code that adds the DNT header is the cache context addition.

+  $variables['#cache']['contexts'][] = 'headers:DNT';

Hi @pianomansam,

So given that line is in the request of #3480626, we could close this issue and merge that instead?

@atwol yes, you absolutely could. However, I wanted to point out that #3480626 undoes much of the work done in this issue.

OK thanks for the information @pianomansam,

I'll (re) close this as fixed, and i'll look at merging #3480626, and any further disucussion in that thread.

Automatically closed - issue fixed for 2 weeks with no activity.