Explicitly register template_preprocess callbacks in hook_theme()

Created a merge request with a proof of concept, basic right now, but already covers BC by only looking for and registering template_ prefixed functions if initial preprocess is not defined.

MR is based on the referenced issue, only relevant changes is the currently last commit: https://git.drupalcode.org/project/drupal/-/commit/a955017c8e669d28297af....

Array key "initial preprocess" obviously up for discussion, I picked this to explain the difference to regular preprocess functions.

I copy pasted the invocation, we can for example use\Drupal\Core\Utility\CallableResolver to also support services.

We discussed this briefly on slack, so I'd like to move some thoughts here.

#3495943: Handle module preprocess functions as OOP hooks is struggling a bit with that and need to introduce the ability for modules to register them as hooks but not for themself but a "template" pseudo-module.

I'd disagree with the characterization that it is struggling, it is solved. I'm not sure I'd call it a pseudo module either, it's a prefix, it's basically the same thing Registry does for it.

As for this issue:
I think it's an interesting approach

use\Drupal\Core\Utility\CallableResolver to also support services.

I think we'd definitely want to do this, it would be a shame to have DI for other preprocess hooks but not these ones.

This does make theme_registry_alter harder to maintain BC for contrib which is not a problem for the original issue, but I'm not sure how many people are moving template preprocess around, and I'm not sure this should be a blocker for this approach, just an observation.

Another thing I see is this relies on call_user_func_array for calling the initial preprocess, I am not a fan of that call, the original approach only uses it for fallback for custom callbacks added in theme registry alter.

If it's decided that this is the direction to go, I really think this should be done together with the other issue, this isn't too much more and if we pull out the template work in the other issue the changes are likely a wash complexity-wise.

This does make theme_registry_alter harder to maintain BC for contrib which is not a problem for the original issue, but I'm not sure how many people are moving template preprocess around, and I'm not sure this should be a blocker for this approach, just an observation.

Yes, it does. As discussed, the question is whether we want to/need to maintain BC. I'm arguing we don't. That we provide an alter hook for something doesn't mean that it has full BC (form_alter for example. form structures have zero BC, you can do whatever, but we do not guarantee anything around that. Or we could never improve a UI). But we need a framework manager decision.

If it's decided that this is the direction to go, I really think this should be done together with the other issue, this isn't too much more and if we pull out the template work in the other issue the changes are likely a wash complexity-wise.

I'm always surprised by just how opposite we sometimes think ;)

I really think those 3 issues should be kept separate. Each introduces its own concept. Reviewing and getting approval for changes is usually much harder than writing the code, and reducing scope helps with that. This requires that template_preprocess() is gone but this and 📌 Handle module preprocess functions Active can then be mostly parallel IMHO. If we all can agree on this being the way forward, your issue can be refactored to ignore template_* stuff entirely, 📌 Move template_preprocess, _template_preprocess_default_variables into services Needs work already moves it out of the current $prefixes discovery loop. There will be a few conflict in Registry and ThemeHandler, but I'd expect them to be pretty easy to resolve.

As discussed, the question is whether we want to/need to maintain BC

Yep as I mentioned I don't think it is a blocker for this if that's the direction we go.

I really think those 3 issues should be kept separate.

I wasn't being super serious, but it would reduce conflicts and the total change isn't too different size wise and let's it be handled at the same time instead of separate preprocess deprecations.

I'm always surprised by just how opposite we sometimes think ;)

It's pretty astounding, but that issue is better for it.

I pushed an update to use the CallableResolver service, so this supports services now, converted template_preprocess_block() as an example.

The blocker is in, rebased.

This has

Agree that this makes sense

I think this is a nice simplification as long as the core team is ok with BC implications. @catch indicated he was ok as long as it is documented on slack.

Agree on the specific array key

I think it should be preprocess initial to match the other preprocess keys.

Documentation updates and change record

I'll create a quick CR.

Follow-ups to convert remaining template_preprocess, possibly a meta with a few child issues and then a deprecation

This can be done once this gets a closer review.

I created the follow up 📌 [pp-1] Convert Template Preprocess hooks to OOP equivalent Postponed: needs info since we need it whether we do this approach or the other.

One thing to decide on is where those callbacks should live.

In my issue, as an example, I converted template_preprocess_container() to a static method directly on \Drupal\Core\Theme\ThemeCommonElements where it's also defined, And I converted template_preprocess_block() and put that on next to hook_theme in the block module, as a non-static method, so it's called as a service, but it's not a hook. I'm not sure if that's a good idea or not and what kind of patterns we want to adopt here.

Thoughts:
* As mentioned in the issue summary, there are a few issues that want to either define value objects for theme_hook definitions or convert the whole thing into plugin-like discovery with classes for each. If/Once we have the that, preprocess could just live on that. But no idea if that will ever happen
* Putting all the template_preprocesss_* callbacks of core include files into ThemeCommonElements wouldn't be a good fit. There are 11 in form.inc, they could be in a preprocess service in the form component. theme.inc has 20, from generic and low-level stuff like container, time and page to things that could live in their own component such as field, local task/action, breadcrumb and there's also some special maintenance stuff, that for example calls explicitly into template_preprocess_page()
* Should module template_preprocess callbacks be in the Hooks namespace? It's kind of nice to see them next to hook_theme(), but it's not really a hook, so kind of weird, especially considering all my complaining in 📌 Handle module preprocess functions Active about converting them to real hooks. Most modules only have a few of them, only a few have more than 5, by far most is views with 17. Maybe standardize on a preprocess service per module? And views and some complex cases could have separate ones.

Putting all the template_preprocesss_* callbacks of core include files into ThemeCommonElements wouldn't be a good fit

also

Should module template_preprocess callbacks be in the Hooks namespace

Honestly I think we don't have to decide that here, but really what I would probably do is break ThemeCommonElements into mutliple files each with their hook part and then roll them up to ThemeCommonElements so we don't change how that is pulled in.

I really think that we just keep the initial preprocess wherever hook_theme is. We have helpers in Hook classes already. We may have some edge cases like views, let's not get bogged down in organization details, we can do a bulk conversion again and then determine organization later.

Adding framework manager to get a decision for template_preprocess_HOOK direction. Either what this issue does or what 📌 Handle module preprocess functions Active

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased.

My personal preference is the approach here over the pseudo 'template' module in the the other issue. I'll put this to a poll of the other framework managers.

Let us know what they say and we can start working towards this if there is agreement!