Problem/Motivation

As discussed in #2241377: [meta] Profile/rationalise cache tags, they add a lot of noise, but we don't have a single real use case for them in core. We have an API, not need to make default assumptions and cache tags that we don't use.

Proposed resolution

Remove them.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#1 remove-block_plugin-cache-tags-2449069-1.patch6.38 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Needs review
FileSize
remove-block_plugin-cache-tags-2449069-1.patch6.38 KB

And here's a patch.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented 
+++ b/core/modules/block/src/Tests/BlockTest.php
@@ -388,19 +384,12 @@ public function testBlockCacheTags() {
-    // The plugin providing the "Powered by Drupal" block is modified; verify a
-    // cache miss.
-    Cache::invalidateTags(array('block_plugin:system_powered_by_block'));
-    $this->drupalGet('<front>');
-    $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS');
-

The code comment alone kind of shows how little sense those cache tags make. How on earth do you "modify a plugin"? change the code? :)

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs review » Reviewed & tested by the community

It is probably so that you could e.g. configure different text for different instances via config or such?

As a block:[foo] is one instance of a block plugin, but the plugin could have something central to apply to all such ...

That said, _if_ a block plugin in contrib wants to do so, they can just add the getCacheTags() method and be done, no need to default for all of those. when its not used in core.

As that:

RTBC, if tests pass

Could use a beta eval, too.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Title: Remove default block_plugin cache tags » Remove default block_plugin cache tags, because they're useless
Issue tags: +D8 cacheability, +Performance

+100 — see #2241377-34: [meta] Profile/rationalise cache tags.

How on earth do you "modify a plugin"? change the code? :)

Exactly!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Removing stuff that we are not using and makes no sense reduces fragility because we have less to maintain and test. Committed 809854e and pushed to 8.0.x. Thanks!

  • alexpott committed 809854e on 8.0.x 
    Issue #2449069 by Berdir: Remove default block_plugin cache tags,...

Status: Fixed » Closed (fixed)

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