Empty block/region detection #597
Description
TL;DR:
We cannot have both these things:
- guaranteed removal of empty regions
- lazy rendering of blocks (including BigPipe, placeholders for caching etc)
The current implementation tries to do both and is broken.
Pick one of those things. Perhaps on a per-block or per-region basis.
Summary
There's a long and tricky core issue about this. Here's a distilled version & how it applies to localgov_base:
-
Drupal renders page templates before individual blocks. That's good - it facilitates BigPipe, and means that mostly stable pages with small amounts of volatile content can be cached effectively.
-
At the time the page is rendered we know what blocks are present, but we don't know what they will contain (if anything). Here is the conundrum: we want to know if a block contains something, because we want to use classes like
has-sidebarinpage.html.twig. But all we have is a placeholder, not actual content. -
If we do nothing we must accept that a block (and a region) might end being empty. In this case "has sidebar" really means "might have a sidebar".
-
Anything more complicated involves choosing between guaranteed detection of "is empty" and lazy building.
Choosing lazy building
This is the default, so we don't need to do anything.
We can mitigate against empty blocks a bit with condition plugins. Conditions are evaluated ahead of time. For example: a view that only produces results for logged-in users can be hidden from anonymous users when it is placed. There is no need to render it and examine the output.
Choosing guaranteed "is empty" detection
This involves sacrificing lazy building.
There are various approaches suggested online. All some variation of the following, done in a hook_preprocess_page():
- find the placeholder for a block
- render that block
- replace the placeholder with what we just rendered
- apply the cache metadata to the page
Points 2/3 render BigPipe useless.
Point 4 reduces cacheability. It's particularly agressive with hard-to-cache or uncacheable blocks: they make the whole page equally hard to cache.
The current solution in localgov_base does steps 1-3 only.
Demonstrating the issue
Here is a block that displays a message when the current time is an even number of minutes.
It's a really contrived example to make it obvious what's going on. But this could just as easily be an uncacheable view, or some other dynamic thing.
Place the block in a sidebar.
final class FunnyBlock extends BlockBase {
// This block can never be cached.
use UncacheableDependencyTrait;
public function build() {
// We only show content during even numbered minutes.
$seconds = strtotime('now');
$minutes = intdiv($seconds, 60);
$is_even_numbered_minute = ($minutes % 2 === 0);
$build = [];
if ($is_even_numbered_minute) {
$build['message'] = ['#plain_text' => 'Even numbered minute.'];
}
return $build;
}
}Expected behaviour with lazy building
- During even numbered minutes, there is a sidebar and text
- During odd numbered minutes, there is a sidebar but no text
- The page template is cachable
The sidebar is shown because we see the placeholder inside the sidebar and deem that has-sidebar is appropriate.
Expected behaviour with guaranteed "is empty" detection
- During even numbered minutes, there is a sidebar and text
- During odd numbered minutes, there is neither sidebar nor text
- The page template is not cachable.
The page is not cacheable because has-sidebar is not cacheable.
Actual behaviour
localgov_base tries to guarantee empty/non-empty detection. But here's what happens:
- Start with an otherwise cacheable page. A node will do.
- Place the block in the sidebar.
- Clear cache.
- Visit the page during an even numbered minute: the sidebar and block appear (good).
- Visit the page again one minute later: there is an empty sidebar (bad).
- Clear cache.
- Visit the page during an odd numbered minute: no sidebar, no block (good).
- Visit the page again one minute later: still no sidebar or block (bad).
Why this fails: in step 4 and 7 we did not apply the cacheability of the rendered block to the page. So the page was cached with whatever has-sidebar value it had at the time. No consideration was made that has-sidebar might change.
Steps forward
Before we can do anything, can we establish how should this theme actually behave?
- Guarantee emptiness detection for all blocks/regions, at the expense of caching
- Don't guarantee it, accept that "has sidebar" means "might have sidebar"
- Do it sometimes? If so, which blocks/regions should get this treatment? Make it opt-in?
- Something else?