Move preprocess functions into NodeThemeHooks

I compared the changes from node.module to NodeThemeHooks with the following commands:

diff -upb \
  <(git show 11.x:core/modules/node/node.module | sed -n '208,256p') \
  <(git show 3532204-move-preprocess-functions:core/modules/node/src/Hook/NodeThemeHooks.php | sed -n '208,256p') \
  >> node-theme-hooks.patch

diff -upb \
  <(git show 11.x:core/modules/node/node.module | sed -n '260,273p') \
  <(git show 3532204-move-preprocess-functions:core/modules/node/src/Hook/NodeThemeHooks.php | sed -n '64,78p') \
  >> node-theme-hooks.patch
  
diff -upb \
  <(git show 11.x:core/modules/node/node.module | sed -n '277,374p') \
  <(git show 3532204-move-preprocess-functions:core/modules/node/src/Hook/NodeThemeHooks.php | sed -n '106,203p') \
  >> node-theme-hooks.patch

Then looking at node-theme-hooks.patch, which I've attached, I can see the only changes are:

• function names
• addition of hook attributes
• indentation
• use of dependency injection

I also grepped for each of the removed functions in case there were string references in comments and found 26 in 11.x and 4 in this branch.

$ git grep -E 'template_preprocess_node_add_list|node_preprocess_html|node_preprocess_block|node_theme_suggestions_node|template_preprocess_node' 11.x | wc -l
26
$ git grep -E 'template_preprocess_node_add_list|node_preprocess_html|node_preprocess_block|node_theme_suggestions_node|template_preprocess_node' 3532204-move-preprocess-functions | wc -l
4

So the following will need to be updated:

$ git grep -E 'template_preprocess_node_add_list|node_preprocess_html|node_preprocess_block|node_theme_suggestions_node|template_preprocess_node' 3532204-move-preprocess-functions
3532204-move-preprocess-functions:core/lib/Drupal/Core/Render/theme.api.php:  // This example is from node_preprocess_html(). It adds the node type to
3532204-move-preprocess-functions:core/lib/Drupal/Core/Render/theme.api.php: * function node_theme_suggestions_node(array $variables): array {
3532204-move-preprocess-functions:core/lib/Drupal/Core/Render/theme.api.php: * '#theme' => 'node__article' is called, then node_theme_suggestions_node()
3532204-move-preprocess-functions:core/lib/Drupal/Core/Render/theme.api.php: * will be invoked, not node_theme_suggestions_node__article(). The specific

Great. I also checked that all of the services injected in to the new class have been used. I found another issue that had injected services but not used them, our phpstan config doesn't detect that. This is RTBC as per #5 and #6.

These are the last hooks you can add the container parameter.

parameters:
  node.skip_procedural_hook_scan: true

This is ready again.

I confirmed the template preprocess are deprecated now. There are literally no uses in contrib for it and we don't have to technically deprecate them, but since people do occasionally call them we've been deprecating template preprocess.

I spent over an hour trying to figure out why marking node as converted breaks core/modules/jsonapi/tests/src/Functional/UserTest.php and another jsonapi test.

I followed these principles:

1 identify the failing test and review for anything obvious
2 checkout 11.x and one by one break the hooks you converted by changing the function name and run the test see if it fails on any (note this can be a false negative)  this can identify any that were converted incorrectly or have some other weird dependency like system_theme did
3 in the branch you coverted on move all procedural functions to node.module and add the proceduralhookscanstop attribute starting from the bottom (this will identify the function that is causing the test to fail)
4 slowly go insane until you realize a hook was documented incorrectly so it converted to the wrong module (views)
5 Add a file_put_contents that outputs gathered hooks, run the test with the skip and without and see what is different

None of these provided a hint.

I even grabbed every function in the three files removed node and grepped for something calling modulehandler invoke with the function name and nothing came back beyond node_mass_update which isn't a real hook.

hook_node_grants is INVOKED by some of the functions, but that shouldn't matter.

I think we move marking this to a follow up.

Oh, a challenge, can't resist a challenge.

It's user cancel, so I assumed it's related to the user_cancel hook in node module and from there it was pretty straight forward to identify.

\Drupal\node\Hook\NodeHooks::userCancelBlockUnpublish uses ModuleHandler::invoke() to call node_mass_update. that's not a hook, but it misuses that to load the function in the include file.

This works:

$this->moduleHandler->loadInclude('node', 'admin.inc');
      node_mass_update($vids, ['uid' => 0, 'revision_uid' => 0], NULL, TRUE, TRUE);

Agreed that this is out of scope, but lets open an issue for it. There is a risk that we break similar calls though.

Oh of course that is why the attribute worked cause I moved it to the module file to make changing position easier.

Btw there is a duplicate about investigating mass_update.

#3524738: Fatal error when passing NULL to Renderer::render() isn't RTBC yet, but I'd suggest we hold this until that's in as that will need to be backported and having template_preprocess_node() converted to OOP will complicate that.

Blocker just got in.

We're missing the trigger error deprecations in the template functions.

Here are the snippets, suggestions are not working for me right now.

@trigger_error(__FUNCTION__ . ' is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. Use \Drupal::service(NodeThemeHooks::class)->preprocessNode($variables); instead. See https://www.drupal.org/node/3533060', E_USER_DEPRECATED);

@trigger_error(__FUNCTION__ . ' is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. Use \Drupal::service(NodeThemeHooks::class)->preprocessNodeAddList($variables); instead. See https://www.drupal.org/node/3533060', E_USER_DEPRECATED);

Didn't mention this in 22, but I reviewed this from scratch there, confirmed all deprecations are correct which is how I caught the missing trigger_error.

Checked all usages align properly
Checked all conversions are correct.

Committed/pushed to 11.x, thanks!