See views_theme_suggestions_node_alter(), and views_theme_suggestions_comment_alter(), also the code in views_preprocess_node() and views_preprocess_comment() actually.
That does stuff on $node->view. Which is assigned by views when displaying a node. But there is no cache context for this, can't really be, so it is not cached correctly.
Related to #2528314: template_preprocess_node() does not add cacheability metadata as well.
The only thing I can think of is to implement hook_entity_build_defaults_alter() and there, based on the view id and display, define a cache key. The obvious downside is that it results in more cache variations.
I'm also not 100% sure that it is completely accurate then. hook_entity_build_defaults_alter() has some limitations, if dynamic page cache doesn't know about those conditions. But that already has to know about the view/display, if not, you already have a problem.
I think as a long term solution, we should deprecate $node->view and everything that depends on it, but there is no way to make that obvious in something like twig theme suggestions debug output, so people will continue to use it.
| Comment | File | Size | Author |
|---|---|---|---|
| #41 | Screenshot 2025-08-22 at 6.38.55 PM.png | 216.66 KB | smustgrave |
| #14 | 2728419-14.patch | 632 bytes | hawkeye.twolf |
Show commandsStart 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, plain diff MR !12998
Comments
Comment #3
catchShould definitely be deprecated.
Also do we have to consider theme suggestions part of the API, or could we potentially remove one in a minor release? Or we could move it to Stable, which ought to be empty in 9.x so counts as deprecated in that sense.
Adding another related issue that's bad for cacheability.
Comment #4
berdirDiscussed with @dawehner. Using hook_entity_build_defaults_alter(), we could add a cache key based on the view. However, that can result in a lot more cache variations for every rendered node. 10 views that show the same node result in $existing_cache_variations (per view mode, per language, per permissions hash) * 10.
Maybe we can have a setting, to enable this, and we only expose the suggestions and the cache key if enabled. Existing sites have it enabled, new ones have it disabled. We show a UI to disable it for sites that don't want it?
Comment #5
catchThe issue is partly that the cacheability is only affected if the suggestion is used, not just because it exists. However because theme suggestions end up just as strings and template names that makes this tricky (both for applying cacheability there and deprecating).
Comment #6
dawehnerConceptually I totally agree that we should NOT have those suggestions in the first place.
Comment #9
dawehnerWhat about throwing deprecations now in case we detect them?
Comment #10
gaëlgYes, if node--view--* templates are "broken" (as Berdir said in https://drupal.stackexchange.com/questions/222785/view-uses-the-wrong-te...), they should not be suggested to themers.
It's even more problematic because these suggestions actually work when there is no cache (typically on a local dev instance).
Everybody seems to agree on this, right ? But how do we deprecate a template suggestion?
Should be just add a "(deprecated)" notice in the Twig debug HTML comment for that suggestion?
Or maybe something clearer to explain that it's already broken (and not just meaning "won't work in D9")?
Or simply remove that suggestion from the HTML comment added by the Twig debug mode? (looks like the best option to me)
Are there other parts of the core code which need to be updated, like PHPDoc maybe? I can to try to work on this if it's quite simple.
Comment #12
2phaWow, this screwed my head for hours today, templates working on local, but not on acquia dev... Ahhh
Comment #14
hawkeye.twolfMight #2511548: Add a "context" array variable to all theme hooks and "#context" array property to all elements to provide optional contextual data open up a path to resolve this issue?
Alternatively, we could consider a different approach than the render cache key method proposed above. The actual node object gets pulled from \Drupal\Core\Cache\MemoryCache\MemoryCache, via EntityStorageBase::getFromStaticCache(). If we sidestep that cache by cloning the node object (or somehow purging the node from static cache before \Drupal\views\Plugin\views\query\Sql::loadEntities() loads it), this issue goes away.
Shall we consider the potential performance implications and/or side effects of such a change? See attached patch.
Comment #16
kristen polPatch from #14 applies cleanly to 9.1.x branch.
Comment #18
berdirI don't understand how #14 would do anything to resolve this issue, this is about render cache conflicts.
IMHO, the only way forward is to deprecate this feature and display a warning if someone relies on this template and remove it in D10.
Comment #19
catchAgreed with #18 - we should deprecate the theme suggestions.
Comment #20
hawkeye.twolf@berdir, I gave a little background in comment #14. I believe you would find that it solves the issue. Now, that's not to say it's a desirable solution, but it would be worth discussing the reasons why.
Comment #21
berdirI've read #14 and i don't see how that helps. The problem is that the logic in views_theme_suggestions_node_alter() is not reflected in the cache information, and cloning the node object doesn't change anything about how that works. Nor does adding more context. For render caching, all information must be known derivable (cache contexts) before the build process begins. This is too late.
What would help is implementing layout_builder_entity_build_defaults_alter() and whenever there is a view, add that information to the cache keys. But in my opinion, the cost of that is simply not worth it, because we'd end up with possibly a lot more cache variations when they are actually not needed for 99% of the sites. That's why I think this feature should be deprecated, we have view modes to have different versions of rendered nodes.
Comment #22
hawkeye.twolf@berdir, I know it's not very intuitive, but like I said, I believe you would find that it actually works. My request is that before we deprecate functionality as being too hard to solve, we actually debate the merits of the solution we do have.
Comment #23
hawkeye.twolfAlso, I should be clear about what specifically #14 fixes: The views-based entity theme suggestions (e.g.,
node--view--*). It may very well create other problems or fail to solve the underlying issue, but #14 does make the templates get picked up.Comment #24
krzysztof domańskiBased on several core contributors these theme suggestions should be deprecated so we need #3159050: Allow deprecating theme suggestions.
Comment #27
amanire commented+1 to deprecate via #3159050, speaking as someone who just sunk a good chunk of time into an approach relying on
node--view--*templates in Drupal 8.In the meantime, can we revise the "Views template files" documentation in
views.theme.inc?https://api.drupal.org/api/drupal/core%21modules%21views%21views.theme.i...
Comment #28
neclimdulIf i'm reading hawkeye's comment in 14 right, he's saying that triggering the clone causes something in the render process to happen different which i guess is populating context's through render wrapping or something. I can't confirm this to be true, that's a very complicated series of events. Assuming it does work, I think the complication kinda provides an answer to the merits of the approach though. Possible performance impacts aside (purposely breaking static caches sort of proves this is going to be an issue), that would be super hard to test, nearly impossible to rationalize, and super brittle in the real world. It seems like changes by modules or customization could easily trigger a break in the careful stacking of processes.
I'm definitely for deprecating. Two reasons
1. This a little landmine waiting to destroy a developers deploy day.
2. I'm not sure the suggestion even makes sense. In the larger picture, the suggestion is applying another display mode on top of display modes. This means on top of the caching problems documented in this issue, there are underlying logic problems with managing fields and stuff. It seems like you want to use node rendering you should probably be using display/view modes for the different renders otherwise you should use one of the other views template methods.
Going to mark this PP on getting #3159050: Allow deprecating theme suggestions since it seems like that's a requirement for this going in and bumping the priority on that since its now blocking a Major.
Comment #30
damontgomery commentedThis should be removed if it doesn't work and can be added back in if a working version is found.
I just spent hours on this, because it's a suggestion and the suggestion is broken.
Multiple display modes with the same view on the same page all get the second "current_display".
Multiple views that display the same node entities get the second display too.
- Create two views that create blocks that render the same entity type in two unique view modes
- Place one of each block on a layout builder page
- Immediately after the second is placed, the first uses the seconds view mode
I tried this with different display modes on the same view and different views.
Comment #31
gaëlgComment #35
gbirch commentedThis issue just destroyed my day, FWIW, in Drupal 10.3 no less.
I have added a bug report with suggested revisions to the documentation as a temporary fix. https://www.drupal.org/project/drupal/issues/3479114
Comment #36
catch#3159050: Allow deprecating theme suggestions is in.
Comment #38
berdirFinally, we can start to get rid of this.
Comment #39
berdirHere's a question: Do we need BC test coverage for a feature that never had any test coverage in the first place?
We have test coverage for the theme suggestion deprecation feature, this would essentially just test that we have deprecated the right suggstions.
To test manually, all you have to do is visit frontpage on standard install profile after creating an article with twig debug on to see this:
Comment #40
catchI think it's OK to just do the deprecation here as long as we have generic coverage of the deprecation mechanism.
Comment #41
smustgrave commentedI use to be the person that assumed everything needs tests and then it ended up just being a ton of removals later on. As mentioned the functionality of deprecating theme suggestions is covered. I've attached a screenshot showing this doing as expected.
Comment #43
catchCommitted/pushed to 11.x, thanks!