Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
node system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
30 Jun 2024 at 22:43 UTC
Updated:
13 Sep 2024 at 03:53 UTC
Jump to comment: Most recent
Problem/Motivation
We are still passing a hardcoded $teaser boolean to node templates about fifteen years after configurable view modes were introduced. This can just be deprecated now.
There are other node template variables to deprecate and remove, but because we haven't done that very much at all yet, limiting this to one very obvious one.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3458183
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:
- 3458183-deprecate-variablesteaser
changes, plain diff MR !8592
Comments
Comment #2
catchComment #4
catchComment #5
andypostNice idea but tests are failed, so there's usage to clean-up
Comment #6
spokjeThe deprecation process of theme-template-stuff is not very clear, (at least to me): https://www.drupal.org/about/core/policies/core-change-policies/drupal-d....
Looks like the current deprecation message isn't added to the
['node']['variables']return ofnode_theme(), but instead is overwriting it, leaving all node-theming logic without the actual node and only the message. Hence the gazillion test-failures.If I look here, we can only deprecate the root level itself and not anything specific below it? https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
If I use that option for this issue, we're getting a deprecation warning _every_ time
node_theme()is used.We _could_ ignore that deprecation warning in
core/.deprecation-ignore.txt, but maybe this would be the time to introduce deprecation at lower levels of theming functions return arrays?Comment #7
catch@Spokje I think deprecating in node_theme() if variables was already in the definition might have been OK, but rather than figure that out, I moved the deprecation to template_preprocess_node() next to where the variable is actually set, which seems like a better place for it anyway.
We have one usage of this in Olivero which I updated in the MR. This shows the limitations of the deprecation because unless we made $variables an ArrayAccess object, we can't trigger anything when it's referenced in preprocess - but this will only be removed in a major update so I think it's OK to do the best we can for now. Maybe a follow-up for $variables as ArrayAccess is worth exploring.
Comment #8
spokjeNice, this even detected the sole usage in Olivero, so I'm more than happy to RTBC this.
I've decided not to create any follow-ups, since this shows we have a working solution for this case and I'm against Death-By-Admin, but if anyone wants to fill out templates and create a new issue, I'm all for it.
Comment #9
catchThis isn't entirely true. The first version of the MR I removed the setting of $variables['teaser] altogether, which was wrong (it needs to stay in and be deprecated), and that broke Olivero which checks bool instead of empty. If I hadn't made the mistake, the Olivero usage wouldn't have been picked up. However the only way we can do that is to trigger a deprecation error when an array key is accessed and the only way I can think of to do that is $variables as ArrayAccess. Maybe we can do that in a follow-up but it shouldn't block IMO, otherwise we can never deprecate anything in preprocess ever.
Comment #11
alexpottCommitted b2109b4 and pushed to 11.x. Thanks!
Comment #12
catch