Deprecate node_reindex_node_search() and move reindexing logic to a hook

Created on 2 July 2025, 15 days ago

Problem/Motivation

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/node/...

The function node_reindex_node_search() is called from Node.php, mixing procedural code with object-oriented code. This doesn't align with modern Drupal practices.

Proposed resolution

Deprecate the node_reindex_node_search() function in node.module.

Move the search reindexing logic to a more appropriate place, into a proper hook such as hook_node_update()

Remaining tasks

  • Deprecate node_reindex_node_search()
  • Move the logic into a hook
  • Remove the call from Node.php

API changes

node_reindex_node_search() will be deprecated.

📌 Task
Status

Active

Version

11.0 🔥

Component

node system

Created by

🇮🇳India virag.jain

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @virag.jain
  • 🇦🇺Australia acbramley

    There is only 1 usage of this function according to http://codcontrib.hank.vps-private.net/search?text=node_reindex_node_sea... and that module doesn't have a release, looks unmaintained.

    I don't think we need a replacement here, we'll just do all the work in a new Hooks file. Have updated the proposed solution.

  • First commit to issue fork.
  • Pipeline finished with Failed
    14 days ago
    Total: 123s
    #537941
  • 🇮🇳India libbna New Delhi, India

    Hey, in #4 I have done the following changes:

    1. Deprecate node_reindex_node_search()
    2. Move the logic into a new public method on a new NodeEntityHooks class
    3. Move the following methods from NodeHooks1 into NodeEntityHooks: commentInsert, commentUpdate, commentDelete
    4. Remove the call from Node.php, move into a #[Hook('node_update')] hook on NodeEntityHooks

    Let me know if everything looks good or if any changes are needed, or if I’ve missed anything. I'm changing the status to "Needs Work" for now so the work can continue in case the pipeline fails or test cases need to be added.

  • Pipeline finished with Failed
    14 days ago
    Total: 255s
    #538613
  • Pipeline finished with Failed
    14 days ago
    Total: 195s
    #538617
  • Pipeline finished with Success
    14 days ago
    Total: 563s
    #538619
  • 🇦🇺Australia acbramley

    Actioned feedback and fixed up linting.

    Only thing I'm not sure on is if we should even mention the new replacement in the CR, it's in a Hooks class so it's a bit weird but it does technically mean there's a way to do it without us having to add yet another service.

  • 🇨🇭Switzerland berdir Switzerland

    I'm generally not very fond of modules implementing their own entity hooks but I think this is a good example for when it makes sense: Optional integration with another module, this allows us to encapsulate and group all related hooks for that in one service. And with ✨ Allow to skip OOP hooks and services for modules that are not installed Active , we plan to have a means for dynamically registering this hooks service only when the search module is installed, something that for now would either require a node_search glue module or custom ServiceProvider code.

    And then with this we also have a real use case in core that we can test it with.

    I think mentioning the replacement in the CR is fine, personally I wouldn't really have bothered with the Why section. Realistically speaking, we plan to deprecate basically all runtime procedural code by deprecating .module files, doesn't really matter where it's called from.

Production build 0.71.5 2024