WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#24429 closed enhancement (duplicate)

Provide a way to deprecate and/or rename hooks

Reported by: ryanve Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Plugins Keywords:
Focuses: Cc:

Description

It would be useful for themes/plugins/core to have a way to rename a hook with a another hook, such that when add_filter($tag, $fn) is called, the $tag is replaced with another tag. Syntactically, maybe:

  • rename_filter($old_tag, $replacement_tag)
  • deprecate_filter($old_tag, $optional_replacement_tag)

Or $tag could be made filterable as add_filter is called. This would add a lot of capabilities:

if ( ! apply_filters('add_filter_args', func_get_args()))
    return false;

Or, still many capabilities, and safer:

if (false === apply_filters('hook_tag', $tag, $fn, $priority, $accept)
    return false;

Consider: #23259

Change History (5)

#1 @nacin
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #10441.

#2 follow-up: @ryanve
4 years ago

#10441 relates but does not duplicate. This one allows one hook to replace another, but does not force warnings. An important reason for this is to help make theme/plugin updates more seamless. Warnings should be opt in, and could be done via deprecate_filter as an option or filter there.

The primary feature request here is to enable a way to modify the $tag at the time a filter is added for it. Other features could build off that. Handling deprecations are just one application. I think my last block is the better one. It's missing a paren, but you get the idea.

#3 in reply to: ↑ 2 @nacin
4 years ago

Replying to ryanve:

The primary feature request here is to enable a way to modify the $tag at the time a filter is added for it. Other features could build off that. Handling deprecations are just one application. I think my last block is the better one. It's missing a paren, but you get the idea.

Wontfix. The API doesn't need this extra baggage. This would slow things down immensely.

Either don't rename filters, or leave the old ones there. No amount of extra code is going to make that easier.

#4 @ryanve
4 years ago

I've read #23259, #14671, and #14280. The performance lost by adding a filter here could be regained elsewhere. I know add_filter runs a lot. The extra capability here seems worth it.

Applying the old hooks in place is not ideal and does not achieve the same effect. It is messy and mixes up the priorities. There currently is no way to perfectly alias a hook b/c one fires before the other. Applying unused hooks is a waste itself. The return false above can stop hooks from being added and thus save performance. They'd never exist in the apply_filters stack in the first place.

Last edited 4 years ago by ryanve (previous) (diff)

#5 @nacin
4 years ago

The idea here is still too heavy, though. If you want to prove me wrong, go profile it using Xdebug + KCachegrind.

The idea is not wholly without merit, but it not only adds CPU cycles, but also complexity to an ultimately very simple API.

Here's a way it could be made lighter:

function register_hook_alias( $alias, $actual_hook ) {
    global $hook_aliases;
    $hook_aliases[ $alias ] = $actual_hook;
}

function add_filter( $tag, ... ) {
   global $hook_aliases;
   if ( isset( $hook_aliases[ $tag ] ) )
      $tag = $hook_aliases[ $tag ];
}

But, that is *still* heavy. Why call isset() another few thousand times per page, when this functionality likely won't be used?

Something like register_hook_alias() could work, but this would need to happen on execution time. Something more like this:

function register_hook_alias( $alias, $actual_hook ) {
   global $hook_aliases;
   if ( isset( $hook_aliases[ $actual_hook ] ) )
      $hook_aliases[ $actual_hook ][] = $alias;
   else
      $hook_aliases[ $actual_hook ] = array( $alias );
}

function apply_filters( $tag, ... ) {
     global $hook_aliases;
     if ( isset( $hook_alises[ $tag ] ) ) {
       // Merge...

But this is still more code for fairly little gain. Core doesn't rename filters, and I need only three fingers to count up the number of core filters that are marked with a // deprecated comment.

I still think this should go through #10441, given it is an established ticket.

Note: See TracTickets for help on using tickets.