Allow filtering the $function parameter of add_filter(), has_filter() and remove_filter()

[MikeSchinkel]

In a ​comment made by someone on a recent blog post of mine entitled "Enabling Action and Filter Hook Removal from Class-based WordPress Plugins" the commenter lamented on the complexity of removing hooks when plugin developers use classes.

Plugin developers who use classes use so many different architectures; i.e. some use static methods, others using instance methods with singletons, other's use class factories, etc. If WordPress were to allow plugin developers to filter the $function value passed to the _wp_filter_build_unique_id() function then our plugins could mask much of the complexity required for the site builder to interact with our plugins. I could for example support the following syntax even though I'm really might be using a Singleton:

remove_action( 'the_content', 'My_Plugin_Main_Class::the_content' );

Of even the following so the site builder doesn't have to know all about my class hierarchy:

remove_action( 'the_content', 'My_Plugin::the_content' );

Then it would just be up to the plugin developer to add some simple documentation to their plugin's README file.

[MikeSchinkel]

Added 'hook_function' hook to _wp_filter_build_unique_id()

[nacin]

This sounds clever, but:

• It proposes a solution at the onset, rather than the problem.

• If the plugin author would need to do something special to support it, what is the benefit to this, over better education of developers to ensure they expose object references (when not just using static methods , which is even easier)?

• This masks the simple callback nature of the second argument, potentially making it more difficult to read a plugin, especially if one is less familiar with WP.

• How would you avoid obvious recursion issues in core? And in the plugins themselves?

• This would potentially slow down an API that needs to be very fast because it is called so often. Worse, performance would degrade further as more plugins opt into these filters.

[MikeSchinkel]

Replying to nacin:

This sounds clever, but:

• It proposes a solution at the onset, rather than the problem.

I thought that's what trac was for, to post code which is a solution by nature?

I sometimes feel I'm damned if I do, damned if I don't...

• If the plugin author would need to do something special to support it, what is the benefit to this, over better education of developers to ensure they expose object references (when not just using static methods , which is even easier)?

Did you read the commenter's comments? His issue wasn't with developers who expose, it was that when they expose it's complicated for someone who is not a full time developer to grok how to work with it.

The problem I'm trying to solve here is one of complexity for the site builder, not the need for plugin developers to do a better job (I myself can already do what's needed to expose those object references, I just blogged about it after all.)

I want to be able to build plugins that are very easy for site builders to use, and this commenter pointed out that if we use classes (he thinks, and currently he's right) that means it's going to be harder for him to work with.

• This masks the simple callback nature of the second argument, potentially making it more difficult to read a plugin, especially if one is less familiar with WP.

True. But then people who are not familiar with WP won't know what add_filter(), has_filter() and remove_filter() actually do before they read the code for those functions.

Maybe alternate solution would be to have anything prefixed with '@' to a special case, or something like that.

• How would you avoid obvious recursion issues in core? And in the plugins themselves?

Good point.

When I started writing this up I was only trying to fix the remove_filter() use case and for that I considered the recursion issue but when I expanded to include more thanremove_filter() I forgot to consider it.

Assuming this would be considered in general then the solution is to disable applying filters for this one hook.

• This would potentially slow down an API that needs to be very fast because it is called so often. Worse, performance would degrade further as more plugins opt into these filters.

True. Well if the we only considered a string and first character were an indicator it would be a very fast check before running the filter. I've uploaded another patch for consideration. With this approach we could support one of these syntaxes:

remove_action( 'the_content', '@My_Plugin::the_content' );
remove_action( 'the_content', '@My_Plugin->the_content' );

And if not would you consider it if we limit the scope to remove_filter() then, as that's where the issue arose?

Better yet, could you read the comments on that post and see if you can come up with a better solution to empower plugin developers to address the commenter's concerns?

[rmccue]

Giant -1 on this. This is adding a layer of abstraction where it's not needed.

---

There's basically four types of callbacks I can hook in:

1. Normal functions: add_action('init', 'myfunction'). These are dead easy to unhook: remove_action('init', 'myfunction')
2. Class methods: add_action('init', ['MyClass', 'method']) or add_action('init', 'MyClass::method'). Again, dead easy to unhook: remove_action('init', array('MyClass', 'method') and remove_action('init', 'MyClass::method')
3. Object methods: add_action('init', [$this, 'method']). This is where it gets harder and more complicated. I'll break it down into the techniques commonly used with this:
   1. $myplugin = new MyPlugin(): This is pretty easy to unhook, since we're almost always in the global scope when loading plugins (by design). global $myplugin; remove_action('init', [$myplugin, 'method'])
   2. MyPlugin::instance(): Again, easy since these are global state (although they're not in the global scope. remove_action('init', [MyPlugin::instance(), 'method'])
   3. new MyPlugin(): This is the main issue, since although the object is instantiated, it's never actually assigned to anything. This is one case where it would be handy to be able to filter the callback, but that's simply a band-aid fix over the real problem, which is that this is sloppy coding that doesn't consider other plugins.
4. Anonymous functions/Closures: add_action('init', function () { /* ... */ }). Again, this is a problem because the function is never assigned to a variable. Also again, this would be a band-aid solution, since this is just sloppy coding.

(Please note: when I said "sloppy coding", I'm talking about in the context of playing nice with other plugins, not saying these things are bad in general.)

So basically, we have two problem callback types. Both of these are a direct result of the plugin author not caring about other plugins enough to give us anything to work with. I personally don't think we need to hold those plugin authors' hands and/or encourage them to do this. Name and shame those who use this, and if that doesn't work, fork their plugin and fix it.

Another issue with this is that callbacks become indirect and may lead to ill-defined behaviour. Using the example from your suggestion (My_Plugin::the_content), maybe you filter this to change it to actually point to [$myplugin, 'the_content'] (where $myplugin = new My_Plugin_Main_Class()). What if I have a separate class called My_Plugin, and I want to hook My_Plugin::the_content() in? Suddenly, the hook has been hijacked from underneath me, and that would be so much worse to debug when anything could be hijacking it.

You said previously:

Did you read the commenter's comments? His issue wasn't with developers who expose, it was that when they expose it's complicated for someone who is not a full time developer to grok how to work with it.

To me, this feels like it's getting more complicated, not less.

[scribu]

So, there are actually 2 aspects raised in this ticket:

1) Being able to filter the callback.

The problem exists, but the solution is overkill. In summary:

Yo dawg, I heard you liked filters, so we put an apply_filters() in your add_filter() so you can filter while you filter!

2) Making remove_action( 'the_content', 'My_Plugin_Main_Class::the_content' ); work. I've opened a new ticket for that: #23265

[MikeSchinkel]

Replying to rmccue:

2. Class methods: add_action('init', ['MyClass', 'method']) or add_action('init', 'MyClass::method'). Again, dead easy to unhook: remove_action('init', array('MyClass', 'method') and remove_action('init', 'MyClass::method')
3. Object methods: add_action('init', [$this, 'method']). This is where it gets harder and more complicated. I'll break it down into the techniques commonly used with this:
   1. $myplugin = new MyPlugin(): This is pretty easy to unhook, since we're almost always in the global scope when loading plugins (by design). global $myplugin; remove_action('init', [$myplugin, 'method'])
   2. MyPlugin::instance(): Again, easy since these are global state (although they're not in the global scope. remove_action('init', [MyPlugin::instance(), 'method'])
   3. new MyPlugin(): This is the main issue, since although the object is instantiated, it's never actually assigned to anything. This is one case where it would be handy to be able to filter the callback, but that's simply a band-aid fix over the real problem, which is that this is sloppy coding that doesn't consider other plugins.

Let's ignore everything else. This is the problem use-case. Why? Because there is no standard way that site builders to remove hooks so there are forced to dive into the plugin code to figure it out, and many of them don't have the necessary skills with PHP to achieve that.

The purpose of these ticket was to get WordPress to give site builders a documented approach to be able to remove hooks no matter how the plugin developer chose to implement the hooks, and to give plugin developers a method to support the documented approach.

To me, this feels like it's getting more complicated, not less.

I'm fine with that. What I'll then ask is how can we alternately address this issue? How can we give site builders a documented approach to be able to remove hooks no matter that you build plugins one way and I build them another? If we can identify that then we can create another ticket to request it.

Replying to scribu:

So, there are actually 2 aspects raised in this ticket:

Sorry, you got it wrong. You identified the two unimportant aspects of this ticket and missed the important one. For the latter see what I wrote to rmccue above.

In summary:
Yo dawg, I heard you liked filters, so we put an apply_filters() in your add_filter() so you can filter while you filter!

Is being disrespectful a pathology with you, or is there some other reason you can't stop yourself from always chosing to be rude and insulting?

[scribu]

How can we give site builders a documented approach to be able to remove hooks no matter that you build plugins one way and I build them another?

We can't, unless we force plugin authors to build plugins a certain way. Flexibility or uniformity: choose one.

[MikeSchinkel]

Replying to scribu:

How can we give site builders a documented approach to be able to remove hooks no matter that you build plugins one way and I build them another?

We can't, unless we force plugin authors to build plugins a certain way. Flexibility or uniformity: choose one.

Where did I say "force?" I said "give plugin developers a method to support the documented approach." There are equivalent scenarios all over Codex where guidance on how best to implement something is given. This should be no different.

[scribu]

Because there is no standard way that site builders to remove hooks so there are forced to dive into the plugin code to figure it out, and many of them don't have the necessary skills with PHP to achieve that.

If they don't have the necessary skills with PHP, why do you assume they have enough skills to know which hook controls which aspect of the plugin? What about complex features that require multiple hooks? example: WP_Query mangling

Anyway, I think I get what you're looking for:

add_plugin_support( 'feature-x' );

remove_plugin_support( 'feature-x' );

[MikeSchinkel]

Replying to scribu:

Because there is no standard way that site builders to remove hooks so there are forced to dive into the plugin code to figure it out, and many of them don't have the necessary skills with PHP to achieve that.

If they don't have the necessary skills with PHP, why do you assume they have enough skills to know which hook controls which aspect of the plugin?

There are two different archetypes here, not one. The 1st is the plugin author with the skills and 2nd is the sitebuilder without. I was asking for a standard way for the plugin author to empower the lesser skilled site builder.

What about complex features that require multiple hooks? example: WP_Query mangling

I'm not visualizing the use-case you are referring to. I'd need a use-case example to understand.

Anyway, I think I get what you're looking for:

add_plugin_support( 'feature-x' );

remove_plugin_support( 'feature-x' );

That would be interesting and could be very useful. But I would see it as too course-grained to address the hook removal use-case.

FYI, the reason I posted this ticket was because of ​this comment thread where the commenter said (emphasis mine):

It’s such a pita to remove actions & filters hooks called from classes. Makes me hate the overzealous use of classes especially in cases when there are no major benefits to use a class to start with. ... Well your solution is a hell of a lot better than hunting through the wp_filters global cowboy style. If this were the standard for all plugin developers that would be a step forward. The problem also lies in the discoverability. If it’s documented, at least a user could potentially find it. The truth is most plugins aren’t going to have a ton of documentation and would a non-savvy user know what to look for? It would require more explanation. ... If it’s about pushing core changes, I would rather see them make remove_action & remove_filter smarter so that you don’t have to reference the class instance in the first place. That would retroactively make all relevant plugins easier to customize without requiring plugin authors to implement workarounds (even pretty elegant ones like yours). Are there any critical objections to that that you could think of? As a self taught dev who’s learning was based mostly around WP projects, I am happy that most code I encountered wasn’t object oriented in the early days. I’ve found OO much hard to grasp initially. Now that I’ve got some years under my belt, I’m comfortable with it (just wrote a class yesterday because it made the most sense) but I still wish to see less of it in plugins because I find it gets in the way a lot of the time. I imagine lots of new users might feel similar. I do think the offical WP docs contribute to that as well (not a ton of examples with classes etc).

So I'm sensitive to people who want to be empowered by WordPress but who are sometimes overwhelmed by it. And that is who I'm ultimately trying to help here. But I don't really need this for my own work and I didn't expect it to be such an issue so if it is I can fight other battles instead.

[scribu]

But I would see it as too course-grained to address the hook removal use-case.

Yes, but you start from the premise that what sitebuilders want to do is "remove hooks". My contention is that that's not what they want to do; they want to stop those annoying share buttons from appearing under each post. Or they want to not have plugin X enqueue a bunch of JS files on every single page.

Back to the topic at hand: There is no quick fix for needlessly complex plugin or theme code; a patch to WP core won't fix it. Neither will a single Codex page.

What we can do is:

a) Encourage devs to make the class instance available, like you did in your blog post.

b) Write more blog posts on how to organize complex plugins, on when classes are necessary and when they aren't etc.

[MikeSchinkel]

Replying to scribu:

But I would see it as too course-grained to address the hook removal use-case.

Yes, but you start from the premise that what sitebuilders want to do is "remove hooks". My contention is that that's not what they want to do; they want to stop those annoying share buttons from appearing under each post. Or they want to not have plugin X enqueue a bunch of JS files on every single page.

Your contention aside, I had one data point (my commenter) to your assumption. One data point is not a statistically valid sample but neither is your assumption.

Back to the topic at hand: There is no quick fix for needlessly complex plugin or theme code; a patch to WP core won't fix it. Neither will a single Codex page.

What we can do is:

a) Encourage devs to make the class instance available, like you did in your blog post.

b) Write more blog posts on how to organize complex plugins, on when classes are necessary and when they aren't etc.

Fine.

[SergeyBiryukov]

Version field indicates when the feature was initially suggested.

[MikeSchinkel]

Replying to SergeyBiryukov:

Version field indicates when the feature was initially suggested.

It must have been mistaken changed as I didn't do it explicitly, sorry about that.