- Issue created by @nicxvan
- πΊπΈUnited States nicxvan
I have attached a list of all of the preprocess functions in core.
- Status changed to Active
6 months ago 1:12am 11 April 2025 - π¬π§United Kingdom catch
π Explicitly register template_preprocess callbacks in hook_theme() Postponed is in.
- π¨πSwitzerland berdir Switzerland
About 110, some already converted now.
I'd prefer splitting this a bit, thoughts:
* One for remaining bits in core/includes? (32, including already convered ones)
* one for views + views_ui (about 25)
* one for system (13)
* one for the remaining ones ? (110 - 32 - 25 -13 = 40)last chunk still seems pretty big, so maybe split out those that have 4 or more on their own (update, image, file). they all also have include files that can then be removed completely.
For modules, what I'd like to do is move their hook_theme() + the preprocess callbacks into a new ThemeHooks or ${Module}ThemeHooks, that does make it a bit bigger though, but otherwise we have to move it again.
- πΊπΈUnited States nicxvan
I'd vote for splitting it along the 4 lines you mentioned.
Let's move them to ${Module}ThemeHooks, having files called the same is already causing issues finding the right file.
Let's hold off on splitting the last chunk until we get a look at it, but if anything obvious pops up we can split those out to a new issue pretty easily.
- π¨πSwitzerland berdir Switzerland
Might need to split this up into more chunks. With deprecations, reflowing of the code and all those references in templates, it's a lot of changes, already at 530 insertions(+), 314 deletions(-) and only did the part of theme.inc that I put in global ThemePreprocess, there's still field templates, pager, breadcrumb, image, menu and more.
- π¨π¦Canada Charlie ChX Negyesi πCanada
Given the standing issue about removing ModuleServiceProvider I would advise to be cautious about introducing another magic named class
- π§π·Brazil fabiorubim740@outlook.com
fabiorubim740@outlook.com β made their first commit to this issueβs fork.
- π¨πSwitzerland berdir Switzerland
There is no magic class name here. It's just on conventions on how hook classes are named
- πΊπΈUnited States nicxvan
Here is where we are working out the naming convention: π [meta] Clean up hook classes in core Active
- π¨π¦Canada Charlie ChX Negyesi πCanada
Sorry for the misunderstanding: I thought you are converting hooks by themes.
- πΊπΈUnited States nicxvan
That's on the roadmap but this is only about template preprocess hook after π Explicitly register template_preprocess callbacks in hook_theme() Postponed
- π¨πSwitzerland berdir Switzerland
Updated summary a bit, added new issues. I've mostly split up those that have functions in include files, to unblock deprecating the 'file' key.
The big one that's left is views.module, that one is scary. the views theme hook is very complex and dynamic and it automatically defines theme hooks for its plugins, so we need to figure out how to define their template preprocess functions. Either explicit definition in the plugins or some kind of logic. Could even consider putting them on the plugin classes directly, but that would require them to be static.
- π¨πSwitzerland berdir Switzerland
I'm a bit unsure how to deal with inc files in modules that (only) contain template preprocess functions and how to deal with them. Right now, I've moved the BC layers to .module and deleted the file. My reason for that was that it allows us to see which .inc files are really left with stuff and I think we do have a BC policy that files and where these functions are is not covered by it. I also thought that might make the BC layer more likely to work, as the .inc file wont' be autoloaded anymore, not from the original definition anyway.
However, there are also some drawbacks to it: While we claim that those inc files and location of functions aren't covered by BC, it can break things in annoying ways, such as kernel tests loading those files. It also makes the merge requests larger, not only do we copy all the function docblocks to the new OOP classes, but we also copy the deprecated ones to the .module file.
In a number of cases, such as template preprocess and helper functions in views_ui, it also feels a bit silly to provide BC for all that, as those are rarely if ever called directly. So I was wondering if we should really always keep them as BC. But I guess so, to me the main advantage is that if we always do it, we don't need discussions on when it's worth it and when not, and the cost of keeping them around is pretty minor, they'll be easy to find and cleanup and we have means now to do with the merge request reviews with the zebra diff.
Any thoughts on all of this?
- πΊπΈUnited States nicxvan
It's tougher than it seems.
My vote is if they are the only findings move them.
If the diff is small move them.
If we're pushing up against limits like update leave them all and let's open a follow up.
- π¨πSwitzerland berdir Switzerland
Summary for the slack discussion with @catch and @larowlan on this (https://drupal.slack.com/archives/C079NQPQUEN/p1752472142632779):
We move the functions, but keep an empty .inc file to not break some edge cases around loadInclude() with phpstan and manual include_once/require once as done in some kernel tests.
- πΊπΈUnited States nicxvan
Views will likely conflict with this: https://www.drupal.org/project/drupal/issues/3535943 π Convert final 4 preprocess hooks in core modules Active
- π¨πSwitzerland berdir Switzerland
It's still a good chunk of modules left, but at this point, most just have 1-3 template_preprocess function and I'm thinking about grouping them. I think I will try to do one group for all those that have a file key, so we have similar things together (like all the deprecated .inc files) and also that should then unblock π Remove 'includes' support from hook_theme() Postponed . And then one other issue for the remaining ones, maybe.
- πΊπΈUnited States nicxvan
Can we keep the git command in the summary?I use it to review each issue and having it here is handy.
- π¨πSwitzerland berdir Switzerland
Certainly, I also dropped file, had this open in an old tab and sometimes browser remembering input of textareas is counter-productive. Restored.