Region Block gets cached too aggressively

We need a test for this, someone up for the job?

@aspilicious: I have been seeing this very behavior on my development build. Switching 8.x-2.3 to dev, and running your patch #3 addresses this issue on my development site. Thanks!

I think this is not a problem of the block content and his caching information. It's a problem that the DsRegionBlock class does not add an additional context for the region block (maybe the viewed entity).

For example we can add an additional context based on the requested url in the DsRegionBlock class:

public function getCacheContexts() {
   return Cache::mergeContexts(parent::getCacheContexts(), ['url']);
}

I don't know for the moment, how we can easy add a context to the entity which is displayed. But I think this would be the right approach to solve this problem.

I started writing the test requested in #3. Attached is a test case for this particular scenario, it seems though that the region block caching-related functionality is broken in more ways than one: the DsRegionBlock::build() function relies on a global which is set in ds_extras_entity_view_alter(). However, an entity view might get cached in the render array, while the block that relies on the static being set is not cached or cached differently, causing the block to often just be empty (global not set, even though you are looking at an entity/view mode where a block region is defined/populated).

This seems to also cause some strange behaviour (which affects the attached test): if you put 2 blocks (one core block, one DS block region) in the left sidebar and view an entity, then return to block placement, move both blocks to the right sidebar and return to the entity, the core block will have moved to the right sidebar as expected, but the DS region block is missing until caches are cleared, after which it suddenly appears in the right sidebar along with the core block.

In order to finish the tests for the caching bug reported here, I think the above caching-related issue needs to be fixed first (either as part of this issue or in a separate issue).

I attempted to fix the issue by using a #lazy_builder to set the ds_region_block global value, but the lazy_builder is firing after the DsRegionBlock::build() invocation. A probably cleaner approach might be to have the block itself determine which node/entity is being viewed, and call entity rendering itself rather than relying on the brittle global?

I discussed this a bit with @aspilicious the other day and yes, the only sane option here is to instead use a block that just displays an entity with a given view mode.

ctools has such generic block plugin that supports any entity type.

I think we need to deprecate this feature in DS, adding a link to this issue and probably apply the patch in 2 for existing sites.

@berdir, @mr.baileys thoughts?

It could still fail sometimes with that patch. It still assumes that the node is built in the same request as well.

If for some reason only the block is invalidated but the node is not, then that information won't exist and the block will be empty.

What is the suggestion for existing D8 sites which already depend on DS block region? Two sites will soon get launched by us (end of April), depend on DS block regions and have caching issues like mentioned above. Those issues occurred after the deployment on the staging where caching is on, locally during development caching was off and worked during development like expected.

How to handle this situation more short term / more mid and long term?

I disabled the DS block region feature and handle those situation with a preprocessor function where I provide certain variables for Twig which I then render in the page template.

I used to pull out the title into a cover region (above ) the content with DS block regions but now do this manually by content type and provide a variable in the page template.

HOOK_preprocess_page(&$variables){
    if (($node = \Drupal::routeMatch()->getParameter('node')) && $node instanceof \Drupal\node\NodeInterface) {
        ////////////////////////////
        // Overview content type
        ////////////////////////////
        if ($node->getType() == 'overview') {
            // prepare cover title
            if($node->field_overview_cover_title->value) {
                $variables['cover_title'] = $node->field_overview_cover_title->value;
            }
        }
    }
}

This works fine for me and there are no more caching issues like I had with DS block regions. Hopefully this helps somebody who used to use DS block regions.

4aficiona2 brilliant idea — thanks for sharing.

@screenack you're welcome ;-)

Here is the corresponding page template (page.html.twig) section. Beside the title I also provide a background image and there is some logic for the front cover classes. Hope this helps too. And a scroll to main content link.

{% if page.cover or cover_text or cover_background_image %}
  <section class="cover {{ is_front ? 'cover--front' : 'cover--default' }}" style="{{ cover_background_image }}">
    <div class="l-contain {{ is_front ? 'cover--front__block--bottom' : 'cover--default__block--bottom' }}">
      <div class="{{ is_front ? 'cover--front__block--bottom__element' : 'cover--default__block--bottom__element' }}">
        {% if cover_text %}
          <div class="text--highlighted--wrapper">
            <h1 class="text--highlighted text--highlighted--transparent text--font--special">{{ cover_text | raw }}</h1>
          </div>
        {% endif %}
        {{ page.cover }}
      </div>
    </div>
    <a class="cta-scroll-down link--no-underline" href="#content--main"><svg class="arrow arrow--big" height="24" version="1.1" viewbox="0 0 24 24" width="24" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><path d="M7.41,8.58L12,13.17L16.59,8.58L18,10L12,16L6,10L7.41,8.58Z"></path></svg></a>
  </section>
{% endif %}

The alternative solution to DS using ctools is not immediately apparent as described in this thread. Berdir can you clarify what you meant in #9? thanks

So, I assumed what was meant by Berdir was to use page manager and the block layout plugin with ctools, however I only experience errors and I abandoned the idea. I ended up using the field as block module and things work as needed.

No, this is not about page_manager. You can place ctools Entity view blocks in the normal block layout as well.

Berdir, thanks for the clarification, however in testing I was not able to get Entity View blocks using the core block layout to render anything in 8.1.0, I doubt I am making an user error, but who knows.

Did you install the ctools module? That one is needed.

aspilicious, thanks for the suggestion. I do have ctools enabled, but looks like "Chaos tools blocks" is not. that is likely the issue I would guess.

Here is a patch that marks the functionality as deprecated with a link to a new handbook page I created.
I hope this is enough to help people with reconfiguring their website.

Here is a link to the new handbook page: https://www.drupal.org/node/2754967

So yeah this is kinda "fixed".