wecodemore / wpstarter

Easily bootstrap whole site Composer packages for WordPress.
https://wecodemore.github.io/wpstarter/
MIT License
244 stars 34 forks source link

Improve handling of MU plugins #58

Closed dnaber-de closed 5 years ago

dnaber-de commented 7 years ago

WPStarter loads MU plugins in sub directories via the WCM\WPStarter\MuLoader\MuLoader that acts on the action muplugins_loaded. This leads to a problem when a mu plugin registers itself an action listener to muplugins_loaded. So instead of handling MU plugins on runtime, what about dealing with them on build-time?

WPStarter could add an additional step that crawls the MU plugin main files and places a file for each directly to the MU plugins directory. This file just contains the plugin header and includes the original mu-plugin file in its sub directory.

Here's an example:

The WPStarter project requires the package acme/acme-mu-plugin which is of type wordpress-muplugin. Composer installer moves the package to wp-content/mu-plugins/acme-mu-plugin/. Now WPStarter creates a wp-content/mu-plugins/acme-mu-plugin.php with the following content:

<?php

/**
 * Plugin Header parsed from acme-mu-plugin/acme-mu-plugin.php
 */

require_once "/absolute/path/to/wp-content/mu-plugins/acme-mu-plugin/acme-mu-plugin.php";
gmazzap commented 7 years ago

WP Starter MU plugins handling is definetively something that needs improvements.

For version 3, I experimented a different approach (available in version-3 branch), but I'm not happy with it.

I like very much the idea of dealing with MU plugins at build time.

What I don't like is to have a "loader" for each installed plugin.

Since Composer was not even a thing a quite common practice that worked well was to have a single "loader" MU plugin that loaded all MU plugins in folders.

Usually such MU plugin used glob or the like to load MU plugins from subfolders of MU plugin folder, but WP Starter could benefit of build routine to have a static array to read file names from.

Something like:

<?php
/*
 * Plugin Name: WP Starter MU plugin loader
 */

// Array of plugin files to load created by WP Starter at build time
$mu_plugin_files = array( ... );

array_walk($mu_plugin_files, function($file) {
   wp_register_plugin_realpath($file);
   include_once $file;
});

Being a static array the loading would be fast and we would also benefit of opcache for even better performance.

Creating this MU pugin at build time from a template would be trivial, the trickiest part would be how to build the $mu_plugin_files array.

Just relying on the Composer type (that's what currently WP Starter does) has proven to be suboptimal and that was the reason I tried a different approach for version 3.

Since WP only allows single files as MU plugins, many developers, even ones inclined to use Composer in their plugins, just give up using folder (and include a composer.json in there) for MU plugins, because of the additional overhead to load them from folders.

The approach I took for what's in version-3 branch is:

1. if a composer.json is there, go to step 1a, otherwise go to step 2.

1a. If the type in composer.json is wordpress-muplugin go to step 1b. if the type in composer.json is wordpress-plugin go to step 1c. otherwise skip the folder.

1b. check for PHP files in the folder. If there's only one PHP file, assume it is the MU plugin file and queue it to be loaded. If there are more files, look for the file who has plugin file header (that's not mandatory for MU plugins) and if found queue it to be loaded. If both 2 previous attempt failed skip the folder.

1c. Being a regular plugin, plugin file header is mandatory, so look for the file who has plugin file header. If found, queue it to be loaded, otherwise skip the folder.

2. Scan the folder for a file who has plugin file header. If found, queue it to be loaded, otherwise skip the folder.

Using above workflow to build the $mu_plugin_files would allow to load from subfolder of MU plugin folder:

Since the (slow) directory scanning happen at build time, we don't affect website performace.

Supporting to load regular plugin as MU plugins allow for some intersting use cases, but also has some gotchas.

In fact, many plugins rely on "activated hook" to do stuff necessary for them to work. We could manage to trigger those hooks comparing the array of plugin being loaded with the activated plugins array in database, but that would not be 100% reliable.

The other alternatives would be:

dnaber-de commented 7 years ago

Just relying on the Composer type (that's what currently WP Starter does) has proven to be suboptimal and that was the reason I tried a different approach for version 3.

What was the issue there again?

Actually it's not part of WPStarter to deal with different types. The Composer installers is responsible for that. IMO WPStarter «just» has to take care about subdirectories of the mu-plugins folder.

  1. if a composer.json is there, go to step 1a, otherwise go to step 2. How could there be a sub-directory in the mu-plugins folder that does not contain a composer.json? The only case I know is that someone created a «virtual» package within its project composer.json file but this is some edge case which I would ignore until someone needs a solution here.

Also I'm not sure whether it's a good idea to support regular plugins to be used as MU plugins. I never faced a situation where this would have been necessary. Just for the sake of simplicity I would leave this out for the moment.

franz-josef-kaiser commented 7 years ago

Also I'm not sure whether it's a good idea to support regular plugins to be used as MU plugins. I never faced a situation where this would have been necessary.

Possible use case: You need to have a DO NOT TURN THIS OFF. EVER! plugin and a multisite installation where site administrators would be able to turn off a plugin.

dnaber-de commented 7 years ago

Being in this situation, why don't you define your plugin as a mu-plugin?

franz-josef-kaiser commented 7 years ago

Your might be more than one file (matching the 1c case from @Giuseppe-Mazzapica ).

gmazzap commented 7 years ago

What was the issue there again? Actually it's not part of WPStarter to deal with different types. The Composer installers is responsible for that. IMO WPStarter «just» has to take care about subdirectories of the mu-plugins folder.

Actually there are 2 issues.

First of all, Composer installers is configurable via extra.installer-paths directive and the Composer type is used just to get the default path.

If WP Starter only rely on Composer "type" directive, it will not be able to recognize a package that is set to be installed in MU plugin folder via extra.installer-paths directive if it has a different "type" or no type at all.

Considering that in WordPress everything that is placed in MU plugins folder is considered a plugin (even the plugin header is optional) it makes sense to me that everything that is installed via Composer in a subfolder of the MU plugin folder is loaded as MU plugin. That includes, but is not restricted to, packages with "wordpress-muplugin" as type.

For me what the WP Starter should do is:

  1. just let Composer install packages according to theirt types and extra.installer-paths directive
  2. check if there's anything that Composer installed in subfolder of MU plugin folder that is eligible to be considered a MU plugin
  3. write a "loader" that loads everything discovered at point 2.

Regarding what is "eligible" to be a considered a MU plugin, I would use the workflow in my previous comment, summarized:

  1. if there's only one PHP file, consider it a the MU plugin
  2. if there's something that contains a plugin header (no matter the Composer type) consider it the MU plugin
  3. if nothing above appllies, loads nothing

The second issue is that, sadly, out there there are a lot of plugins that does not support Composer at all, i.e. don't have a composer.json.

For MU plugins it is even more true, because WordPress allows only single files as MU plugins, but having a composer.json makes sense only if the package is placed in a separate folder.

The workflow suggested above in this comment and in the previous comment, will work for those MU plugins without support for Composer. They will need a combination of a custom Composer repositories entry + extra.installer-paths directive, but IMO that is still better to maintain a fork just to add Composer support.

Also I'm not sure whether it's a good idea to support regular plugins to be used as MU plugins. I never faced a situation where this would have been necessary.

I was in that situation, more than once actually. Last time was when I wanted to install https://github.com/bueltge/Remove-Comments-Absolutely.

The plugin is a regular plugin (containing more than one file that was a forced choice). However, that is something that regards the configuration of the whole network, and I did not wanted the client (who has admin access) to mess up with it.

This was just the last time it happened, but many times I felt the need to use regular plugins as MU plugin. Expecially when network installation is used to build multi-language site, IMO it makes a lot of sense to force anything about site configuration at network level.

Considering that the official WP repo contains only regular plugin, there are a lot of things that I would use as MU plugins, just to name very few:

Being in this situation, why don't you define your plugin as a mu-plugin?

When I write the code to solve the situation @franz-josef-kaiser pointed out, and the solution is site-specific I put the code in the "website" repository (that is the repo that requires WP Starter). In this way I don't need to require it in any way. If the code is reusable, across websites I wrote a Composer package, not a plugin nor a MU plugin, but something that just goes in vendor folder and that is available when autoloader is loaded.

So the issues stands for plugins that already exists, very often third party, which I don't control so I can't change from regular plugin to MU plugin.

Just for the sake of simplicity I would leave this out for the moment.

I agree that support regular plugin as MU plugin has some issues, expecially the avoidation of activation/deactivation hooks.

However, supporting regular plugin as MU plugin but doing nothing in the regard of support of activation/deactivation hooks, (just clearly stating in documentation that plugins that rely on activation/deactivation hook will not work) we would not actually add any complexity, in fact the necesasary code in WP Starter will be exactly the same, regardless support of regular plugin.

I think that if a developer is able to use Composer with custom installer paths to install a regular plugin as MU plugin, will be probably able to understand when a plugin relies on activation / deactivation hooks.

gmazzap commented 5 years ago

This issue is probably not going to happen for v2 series.

And it is addressed in version 3.

I honestly changed my mid, and turned to agree with @dnaber-de

Current v3 implementation only targets only packages with MU plugin type ignoring regular plugin. I've seen library doing this (one by @felixarntz) people can use those if they want.

I am doing a bit of cleanup of issues to move forward with shipping of version 3 so I am going to close this because it will not happen for v2 and done for v3.