Make WordPress Core

Opened 8 years ago

Last modified 5 years ago

#37656 new enhancement

Improve plugin bootstrapping processes

Reported by: flixos90's profile flixos90 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch needs-testing 2nd-opinion
Focuses: multisite Cc:

Description

I recently thought about if we could make bootstrapping plugins easier and take away some common tasks that (should) happen in every plugin. It would also be nice to have a general plugin class registry.

What I was thinking of is to introduce an abstract class WP_Plugin that plugin developers can extend for their plugin's main class. Then they would call a new function register_plugin( __FILE__, $plugin_class_name ) to register that class with WordPress. We could take some regular processes away from the developer and, by doing that, also make sure that they don't implement it the wrong way. For example, we could take care of plugin installation routines: If the class implements a static method install(), the base class WP_Plugin would register an activation hook to an internal function that takes care of whether the plugin is activated network-wide. The actual install() method would only contain those steps necessary for the setup on the current site (WP_Plugin would take care of switching sites as appropriate). Many plugin developers overlook Multisite in their setup routines, causing the plugin to only install on the main site although being network-activated. We could also deal with other tasks, like hooking the bootstrap() method of the plugin class in plugins_loaded or muplugins_loaded (detected by the base class).

I think this whole concept could improve the way plugins initialize themselves. It would not be mandatory (since several plugins don't even use classes), but it would become a best practice. This is just an idea that I wanted to throw on Trac to discuss about it. If we get to the point that we agree this is a good idea, we would need to come up with actual details (of which I don't have any yet).

Attachments (1)

37656.diff (2.9 KB) - added by flixos90 8 years ago.

Download all attachments as: .zip

Change History (16)

#1 @swissspidy
8 years ago

Your suggestion reminds me of WP_Widget. Shouldn't a WP_Plugin rather do similar things as the already existing WP_Theme class? Otherwise this would be very confusing to developers.

#2 @flixos90
8 years ago

@swissspidy: Good point. I think it might be possible for WP_Plugin to do both. We could first develop it in the same way as WP_Theme, but in addition add a method like set_main_class() which would be called from the above register_plugin(). Then WP_Plugin would take care of what I described. The only difference would be that the class passed wouldn't extend WP_Plugin then - instead it would need to implement an interface like WP_Plugin_Interface (not sure about that name, but that's all that came into mind now).

#3 follow-ups: @swissspidy
8 years ago

A WP_Plugin class has been previously mentioned in #22256 and brought up with a patch in #21883, but only in regards of hooks and filters (i.e. $GLOBALS['wp_global_hooks'] = new WP_Plugin();. With #17817 in contention, that approach would need some more tweaking. Also, the class name isn't ideal too because of WP_Theme. Using WP_Plugin for hooks but themes can use hooks too? Confusing.

Starting small has probably the highest chance of getting this into core, i.e. a WP_Plugin class analogous to WP_Theme. Forgetting #21883 and register_plugin() in the beginning.

While register_plugin() and an additional interface sounds interesting, I do not think anyone would use it because of backward compatibility with older WordPress versions. Who wants to basically maintain two versions of their plugin?

#4 @jdgrimes
8 years ago

FWIW, here is what I use in WordPoints to perform install, uninstall, and update. Takes care of multisite, network vs per-site activation, etc. (It really needs to be reworked into separate classes for each of these actions [uninstall/install/update].) But I think really adding anything like this would be for a separate ticket. As @swissspidy said, this has to start small if it is going to become part of core.

As far as making WP_Plugin hold the plugin header data as WP_Theme does for themes, I have a proposal to make in regard to that. What if instead of having to read the file to parse the plugin headers, the header data was passed directly to a function?

<?php
WP_Plugins::register(
   '
   Plugin Name: Foo
   ...
   '
);

The cool thing is that this will still work with the file parsing, since get_file_data() doesn't specifically look for this information only within PHP comments. But it also allows us to get the raw headers without having to read the files at all. So when the plugin is active, we can parse this string passed directly to our function to get the plugin data, and we can still read it with get_file_data() when the plugin is inactive. (I already do this for modules in WordPoints.)

#5 in reply to: ↑ 3 @jdgrimes
8 years ago

Replying to swissspidy:

While register_plugin() and an additional interface sounds interesting, I do not think anyone would use it because of backward compatibility with older WordPress versions. Who wants to basically maintain two versions of their plugin?

Sometimes this kind argument makes sense for a feature, and sometimes it doesn't. Just because plugins might not be able to take advantage of a feature right away doesn't mean that it isn't core material. There was a time when plugins couldn't use the action/filter API because of back-compat. :-) Same goes for every single other thing introduced to core. However, I do see your point that asking a plugin to implement an interface means that they would have to maintain two different classes, since the interface is required, but in older versions of WordPress wouldn't exist. It isn't a feature that can be wrapped in an if version check, it's all or nothing.

Or is it?

<?php
class Foo_Plugin {}

if ( interface_exists( 'WP_Plugin_Interface' ) ) {
   class Foo_Plugin_With_Interface implements WP_Plugin_Interface {}

   register_plugin( __FILE__, 'Foo_Plugin_With_Interface' );
}

That said, I agree that this has to be done slowly and by increments.

#6 @jdgrimes
8 years ago

Sorry for posting yet another comment, but I wanted to say that all of the plugin bootstrap doesn't need to go into a single class. WordPress can help plugins un/install/update themselves better, etc., without a WP_Plugin class, by adding other helper classes that plugins can use to aid them in performing these and other tasks. Really, each part of this bootstrap (if it ever materializes in core) should be its own class. It shouldn't all be stuffed into a giant monster. But each part could be integrated with a WP_Plugin class or the like as it evolves. In other words such a bootstrap should be composed of multiple separate APIs.

#7 in reply to: ↑ 3 @flixos90
8 years ago

  • Focuses multisite added
  • Summary changed from Improve plugin bootstrapping processes with `WP_Plugin` to Improve plugin bootstrapping processes

Replying to swissspidy:

Starting small has probably the highest chance of getting this into core, i.e. a WP_Plugin class analogous to WP_Theme. Forgetting #21883 and register_plugin() in the beginning.

While register_plugin() and an additional interface sounds interesting, I do not think anyone would use it because of backward compatibility with older WordPress versions. Who wants to basically maintain two versions of their plugin?

Tbh I would like this ticket to go more into the direction of the latter. I agree that WP_Plugin shouldn't be used as the class name for this (because like you said it should rather have parity with WP_Theme), however I think there are two different tasks here which are not really related to each other. WP_Plugin as we talked about it now should be handled in a separate ticket.

About this ticket, maybe we can find a way to improve plugin setup processes in a way like @jdgrimes mentioned. I actually like the idea of handling network-wide activation in Core. Maybe it could be like the following:

  • if plugin is activated network-wide, ...
    • detect whether the plugin has hooked into a (new) action activate_{$plugin_basename}_for_site; if it has, automatically iterate through the sites in the network (only if not wp_is_large_network( 'sites' )) and run activate_{$plugin_basename}_for_site on each site
    • in addition, run another (new) action activate_{$plugin_basename}_for_network which the plugin developer can hook into to perform network-wide activation stuff (probably a minor number of plugins needs that, only those that actual deal with multisite stuff)
  • if the plugin is activated on a single site, ...
    • only run the activate_{$plugin_basename}_for_site hook

The existing activate_{$plugin_basename} hook would stay completely unaffected by this. It would still run as before. The new ways would become the new best practice though.

Deactivation could work similarly. For uninstallation we would need to think a bit more on what's the best way to implement a similar solution in Core or whether it should remain as is for now.

@flixos90
8 years ago

#8 @flixos90
8 years ago

In 37656.diff I implemented my proposal from above for activation hooks, as a reference of what it could be like.

#9 @swissspidy
8 years ago

Created a new ticket for a "simple" WP_Plugin class, see #37677.

#10 @swissspidy
8 years ago

@flixos90 I don't know if there are any hooks in core using the 'abc_' . $foo . '_xyz' notation. If there aren't, 'abc_xyz_' . $foo should be preferred.

#11 @flixos90
8 years ago

@swissspidy: Good point. Searching through the codebase, I found an example "auth_post_{$post_type}_meta_{$meta_key}" (just introduced in 4.6), so I guess it's fine. :) Also I think having the plugin basename not at the end of the hook name makes sense semantically in this case.

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

#13 @flixos90
8 years ago

Note: If we do this, we need to make sure to use interpolation for the hook names (see 38307).

#14 @MaximeCulea
5 years ago

#41233 was marked as a duplicate.

#15 @desrosj
5 years ago

  • Keywords has-patch needs-testing 2nd-opinion added
  • Milestone set to Awaiting Review

Readding a milestone value that was removed.

Note: See TracTickets for help on using tickets.