Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
public function getCacheContexts() {
$contexts = [];
// By definition arguments depends on the URL.
// @todo Once contexts are properly injected into block views we could pull
// the information from there.
$contexts[] = 'url';
Currently, adding an argument to a view makes the view cacheable per url even if the argument value depends only on a single query argument value.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #42 | 2514784-42.patch | 7.38 KB | malcolm_p |
| #24 | 2514784-23.patch | 2.7 KB | mparker17 |
Issue fork drupal-2514784
Show commands
Start 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 #1
olli commentedComment #2
olli commentedComment #3
dawehnerYeah right, ideally for blocks this is not true, but for now, this is the best thing we can do.
Do you mind adding a test coverage for that?
Comment #5
lauriiiThe test failures seems to be related to changed cache contexts. We should also remove the comment and @todo. Also manual testing would be good.
Comment #6
wim leersComment #7
olli commented- Fixed test failures
- Removed the comment and @todo
Manually tested
1. Installed standard with latest head
2. Created article with a tag
3. Visited taxonomy/term/1 and checked that X-Drupal-Cache-Contexts header included 'url'
4. Re-saved taxonomy_term view
5. Visited taxonomy/term/1 and checked that X-Drupal-Cache-Contexts header didn't include 'url' but 'url.query_args.pagers:0'
Do we also need a hook_update_N() here that saves all views?
Comment #8
jibranI think so yeah. Not all though just the one using arguments.
Comment #9
jibranGoing to write an update hook.
Comment #10
olli commentedHere's views_update_8001(). I think we also need to update existing views using arguments.
Comment #11
olli commented#9: Great! I hope #10 helps.. Manually tested that and it seemed to work.
Comment #12
jibranWow nice x-post. Needs update hook test now. interdiff is against 7.
Comment #13
olli commentedWhy only enabled?
#12 seems to work but I got "mb_strtolower() expects parameter 1 to be string, array given Unicode.php:331" with drush updb.
Comment #14
jibranOh yeah we can remove the enable check. Nice catch.
Maybe mb_strtolower() error is because the value of
display.*.display_options.arguments.*is an array not string.Feel free to work on this. You can copy the
\Drupal\system\Tests\Update\UpdateSchemaTestto test the hook. If you want to carry on with your patch that's fine by me as well.Comment #16
jibranComment #17
jibranI recreated the patch. If we'll get this in before #2464427: Replace CacheablePluginInterface with CacheableDependencyInterface then we don't need an update hook. There are is a test fix so we already have tests for this. No interdiff because I re-created it.
Comment #18
wim leersWhy would we not need an update hook if we get this in first? Views config entities in HEAD already have cacheability metadata stored/exported.
Comment #19
dawehnerWell, we would have to resave the cache contexts of the view, so its reflected before rendering for the query.
Comment #20
olli commentedComment #23
mparker17Re-rolled the patch in #17. There was a merge conflict in
core/modules/views/src/Tests/Plugin/DisplayPageWebTest.php:... which I resolved to...
Comment #24
mparker17I should probably upload that re-rolled patch...
Comment #25
dawehnerPuh, this could break existing code out there which maybe relies on it?
Comment #33
malcolm_p commentedI've rerolled this for 8.5.x, were there any outstanding issues blocking this and the linked QueryParameter patch from making it into core? It seems like all of the argument related views plugins are now setting their own cache contexts.
Comment #34
dawehnerComment #36
catchComment #39
malcolm_p commentedRe-roll for 8.9.x.
Comment #41
malcolm_p commentedOne more patch with updated configs for
Drupal\KernelTests\Config\DefaultConfigTest::testModuleConfigand adding a hook_post_update based on #12.Comment #42
malcolm_p commentedAnd once more to fix the deprecation in the post_update hook.
Comment #48
dieterholvoet commentedComment #49
smustgrave commentedLast patch seems for 8.9?
Comment #52
chrisolofMerge request 12896 opened against 11.x. It contains the changes from the patch in comment 42 with a slightly improved post-update hook (param and return types declared + lower memory consumption by not loading all of the site's views in one go).
Needs review.
In our site this issue was contributing to render cache pile-ups for teaser/search result-type entity displays that lean on views for certain content (content that does not actually vary by URL).
Comment #53
smustgrave commentedStill needs a summary update.