Introduce a hook for individual plugin loaded

[Rarst]

The current plugin load during core boot is essentially a series of include firing:

// Load active plugins.
foreach ( wp_get_active_and_valid_plugins() as $plugin ) {
        wp_register_plugin_realpath( $plugin );
        include_once( $plugin );
}
unset( $plugin );

Currently it is exceptionally hard to fire off arbitrary code between these includes. The closest I got over years is the following code, which I consider highly unstable: ​https://gist.github.com/Rarst/c32575ffc26df59a45c0

Distinguishing between load events of individual plugins is highly desirable for performance troubleshooting. In current state it is defacto impossible to troubleshoot with PHP code, requiring manual toggling of plugins or low level PHP profiler set up.

Adding individual plugin load event will make it much easier to profile and diagnose performance issues at this load stage.

Can be as simple as:

        include_once( $plugin );
        do_action( 'plugin_loaded', $plugin );

[schlessera]

Why not go all in and make it a flexible system that can be the basis of additional work?

// Pre action to allow for timing hooks to be set,
// debugging information to be initialized, etc...
// Could also allow for the loading to be skipped.
// Dangerous hook, though, needs careful consideration.
$should_load = apply_filters( 'pre_plugin_loaded', $plugin );

// Actually load the plugin.
$should_load && include_once( $plugin );

// Post hook to catch timings, debugging state, etc...
do_action( 'post_plugin_loaded', $plugin );

// Named hook to actually have a proper chance of building a simple
// dependency resolution mechanism.
$plugin_slug = some_method_to_efficiently_retrieve_plugin_slug();
do_action( "plugin_loaded_{$plugin_slug}", $plugin );

As an example for the usefulness of the named hook, such a very simple system has been used in WP_CLI to automatically resolve dependencies between commands. The basic principle would be applicable as well here. If a plugin knows it depends on jetpack, it can check whether jetpack is available when loading, and if not, it can postpone its loading by attaching its own loading process onto the plugin_loaded_jetpack hook.

[Rarst]

Preventing plugins from load is already possible (if not too cleanly) by filtering the option with active list.

To be clear while dependencies are definitely a problematic area, my prime focus with this suggestions is performance troubleshooting and enabling something that is both valuable and low hanging for it.

I don't see (at the moment) anything critical for inter–plugin interactions that cannot be handled on long established plugins_loaded hook, signifying complete load of them.

[schlessera]

Simplest solution to the issue

[schlessera]

I uploaded a very simple patch that immediately solves the raised issue.

As regards to what I had suggested as an enhancement, I will work on a bigger issue/patch that makes one proposal of solving plugin interdependencies, just to show how that could potentially work.

[schlessera]

I've created a new ticket for the suggestion I made earlier in here at #43882. This way, discussing the more elaborate mechanism does not block us from just committing the simple fix we have in here.

[Rarst]

I am not sure why has-patch was dropped here, 41346.diff is valid and exactly what I suggested.

[johnbillion]

@Rarst The patch needs a docblock with description etc

[SergeyBiryukov]

The new hook needs to be documented as per the ​documentation standards.

[Rarst]

I'll take care of documentation, cheers. :)

[ChriCo]

Replying to schlessera:

// snip ...
do_action( "plugin_loaded_{$plugin_slug}", $plugin );

Isn't it better to have

do_action( "plugin_loaded", $plugin, $plugin_slug );

? Otherwhise you would need to hook into {n}-Plugins. Instead you can hook into one action and have all possibilities.

[Rarst]

Replying to ChriCo:

Replying to schlessera:

// snip ...
do_action( "plugin_loaded_{$plugin_slug}", $plugin );

Please note that's not what the current patch actually suggests, it's in fact close to your snippet. :) I would be against dynamic hook as well, they are a mess.

[Rarst]

I've added inline documentation and for consistency respective hooks of MU and network plugins as well.

[Rarst]

Inline documentation and hooks for mu and network plugins as well.

[SergeyBiryukov]

In 44344:

Plugins: Introduce actions for individual plugin load events:

• plugin_loaded: Fires once a single activated plugin has loaded.
• mu_plugin_loaded: Fires once a single must-use plugin has loaded.
• network_plugin_loaded: Fires once a single network-activated plugin has loaded.

Props Rarst, schlessera.
Fixes #41346.

[desrosj]

I am working on a dev note for these new action hooks and came across something we should fix.

The inline documentation for the three new hooks says that the plugin basename is passed. In the current state of the code, this is not correct. The full path to the plugin is actually passed instead. At a minimum, the inline docs should be updated to accurately reflect that the full path is passed.

While I don't necessarily think that passing the full path is bad, I don't know that it is the most useful value to pass to this action. In writing code examples for the dev note, I realized that targeting a specific plugin in that action hook is not as straightforward as it could be.

For example, if I wanted to target code to only run when "myplugin" was loaded, I believe that the best way to really write a conditional to target that plugin in the current state would be as follows:

if ( false !== strpos( $plugin, 'myplugin/myplugin.php ) )

While not terrible, it is not ideal. The full path could be any number of variations baed on server configuration and file structure and the plugin directories can be changed by defining constants before core to name a few of the issues that could come up. The following would be a much better approach:

if ( 'myplugin/myplugin.php' === $plugin )

Thinking this through, there are a few options. First is to update wp_get_mu_plugins(), wp_get_active_network_plugins(), and wp_get_active_and_valid_plugins(), to accept an optional parameter to return both the full path and the shorter plugin name. This seems too risky, though. Especially this late in the release cycle, as these functions have been around for quite a while.

The second option is to just reverse the string concatenation performed in those functions before passing the value to the hook. Attaching a patch with that approach for feedback.

[desrosj]

I would also be ok with not passing the full path if there is agreement that it is not particularly useful.

[Rarst]

Good catch! Somehow I was sure those are basenames, though now that you pointed it out obviously they need to be full paths to be correctly included. Hashtag embarrassed.

As a note on your patch the plugin_basename() should probably be used for consistent result and normalization of path.

However I question do we really care that path is full? Effectively it's just more complete set of information.

Also I need to point out that folder/filename.php is not a reliable signature of plugin. There are scenarios where plugin is defacto installed in a different folder name from what original developer intended. That's why the best practice is to always calculate actual basename on plugin boot, rather than hardcode it.

So providing basenames can be somewhat encouraging to hardcoded checks.

So I suggest we just stick with full path and change inline documentation accordingly. If basename is needed it can be extracted out of full path as necessary by a callback.

[desrosj]

Continuing to pass the full path and updating the documentation is fine with me!

[peterwilsoncc]

Updated docs to clarify full path is passed as argument.

[peterwilsoncc]

In 44692:

Plugins: Update docs for individual plugin loaded actions.

Corrects documentation to indicate the full path is passed to each action.

Fixes #41346.

[desrosj]

The dev note has been posted: ​https://make.wordpress.org/core/2019/01/23/miscellaneous-developer-focused-changes-in-5-1/