[wp-trac] [WordPress Trac] #46635: Improve identifying of non–trivial callbacks in hooks

[WordPress Trac]

#46635: Improve identifying of non–trivial callbacks in hooks
-------------------------+-------------------------------
 Reporter:  Rarst        |       Owner:  (none)
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Awaiting Review
Component:  General      |     Version:
 Severity:  normal       |  Resolution:
 Keywords:               |     Focuses:  coding-standards
-------------------------+-------------------------------

Comment (by giuseppe.mazzapica):

 Hi, author of Brain Monkey here.

 I honestly think this is a much needed thing, and I have experimented with
 this also using a different approach here: https://github.com/inpsyde
 /objects-hooks-remover

 where I introduce a dedicated API:

 * `remove_object_hook`
 * `remove_closure_hook`
 * `remove_class_hook`
 * `remove_instance_hook`
 * `remove_invokable_hook`

 The idea behind is similar, but I felt that a dedicated API would be a
 better fit for a production library (opposed to Brain Monkey that should
 never run in production).

 Now that I think about it, in case of core merge the string identifier
 proposed by Andrey would probably a better fit, so that thousands of
 developers don't need to learn a new API.

 The main issue with both my approaches is that quite expensive operations
 has to be made to properly identify objects, and especially closures.

 But core could do something I could not do: to change how hooks are
 stored.

 In fact, alongside `'function'` and `'accepted_args'`, the method
 `WP_Hook::add_filter` could store a predictable string for the hook, in a
 form similar to the one used by Brain Monkey or proposed here by Andrey,
 this way, when checking for hook existence (`WP_Hook::has_filter()`) or
 when removing hooks, it would be possible to compare the wanted callback,
 provided as string identifier, with this stored identifier, making the
 whole operation simpler and faster.

 This would not pose backward compatibility issues, because for plain
 functions and static methods the identier would be identical to what
 returned by `_wp_filter_build_unique_id`, and in case a non-string is
 passed to `remove_filter` or `has_filter`, `_wp_filter_build_unique_id`
 could continue to be used.

 Moreover, an additional parameter could be added to `add_action` and
 `add_filter` allowing a way to provide a predictable unique identifier for
 the callback.

 For example, `add_action` signature could become:

 {{{#!php
 <?php
 add_action(
   string $tag,
   callable $function_to_add,
   int $priority = 10,
   int $accepted_args = 1,
   string $indetifier = null
 )
 }}}

 and developers could do:

 {{{#!php
 <?php
 add_action( 'save_post', function ( $post_ID, $post ) {}, 42, 2,
 'my_plugin_save_post_cb' );
 }}}

 and then:

 {{{#!php
 <?php
 remove_action( 'save_post', ':my_plugin_save_post_cb', 42 );
 }}}

 I'm using a colon as prefix to avoid conflicts with a real functions,
 because no function name can contain a colon, so that make clear that I
 want to use an identifier, being completely backward compatible.

 In case no identifier is provided when adding the hook, WordPress could
 create an identifier using the "Brain Monkey" way.

 Two things to consider: first should be misured the impact on memory of
 adding an additional string for each hook callback in an average WP
 installation (which does not include just core).

 Second, to distinguish closure by parameters requires heavy use of
 Reflection so it is probably worth to measure performance and see if it
 make sense. In the worst case, having a single string identifier like
 `"function()"` would be better than nothing, expecially if developer would
 have a way to provide a custom identifier as proposed above. Even because
 my experience says that used parameters names are very often the same for
 all the claosures added to the same hook, so they don't add much in making
 a distinction.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46635#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform