- Issue created by @catch
- Status changed to Needs review
2 months ago 10:56pm 30 June 2024 - Status changed to Needs work
2 months ago 11:45pm 30 June 2024 - π«π·France andypost
Nice idea but tests are failed, so there's usage to clean-up
- π³π±Netherlands Spokje
The 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. 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 incore/.deprecation-ignore.txt
, but maybe this would be the time to introduce deprecation at lower levels of theming functions return arrays? - Status changed to Needs review
2 months ago 12:16pm 1 July 2024 - π¬π§United Kingdom 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.
- Status changed to RTBC
2 months ago 12:22pm 1 July 2024 - π³π±Netherlands Spokje
Nice, 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.
- π¬π§United Kingdom catch
Nice, this even detected the sole usage in Olivero
This 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.
-
alexpott β
committed b2109b4e on 11.x
Issue #3458183 by catch, Spokje: Deprecate $variables['teaser']
-
alexpott β
committed b2109b4e on 11.x
- Status changed to Fixed
2 months ago 2:47pm 2 July 2024 Automatically closed - issue fixed for 2 weeks with no activity.