TGMPA / TGM-Plugin-Activation

TGM Plugin Activation is a PHP library that allows you to easily require or recommend plugins for your WordPress themes (and plugins). It allows your users to install, update and even automatically activate plugins in singular or bulk fashion using native WordPress classes, functions and interfaces. You can reference bundled plugins, plugins from the WordPress Plugin Repository or even plugins hosted elsewhere on the internet.
http://tgmpluginactivation.com/
GNU General Public License v2.0
1.75k stars 431 forks source link

Check if did_action function exists #689

Closed terence1990 closed 6 years ago

terence1990 commented 6 years ago

This is an edge case but when using frameworks such as Themosis or Bedrock, these work by loading up dependancies at root level before Wordpress is instantiated, along with any composer autoload files - so functions like did_action in your autoload file do not exist yet, and throw a fatal error.

The way these frameworks work is to load up instantiation of dependant functions and processes through ServiceProviders on boot, and it here that we would call load_tgm_plugin_activation, and not in the composer autoload.

See below ServiceProvider we are using for TGMPA integration with our framework which works nicely when this pull request is applied:

https://github.com/wp-kit/tgmpa-integration/blob/master/src/Tgmpa/TgmpaServiceProvider.php#L22

terence1990 commented 6 years ago

hey @GaryJones i'll make that change, actually ironically in Themosis ABSPATH does get defined before composer dependancies

terence1990 commented 6 years ago

@jrfnl so in Themosis ABSPATH gets defined before composer dependancies which is fine and natural, and we need all of the classes that are being defined in this file, just not the action to be added at this stage - so it's better to wrap the actions in the condition as we have done and not move the conditions anywhere else.

I have added the check for both did_action and add_action

jrfnl commented 6 years ago

I'm still not keen on the current proposed implementation. As I said before: If an additional check like this would go in, I feel it would be cleaner to combine it with or replace the ABSPATH check at the top of the file

In your Themosis implementation, you would then replace the code duplication you have in https://github.com/wp-kit/tgmpa-integration/blob/9b071552d895c8c0891bb6439a23d63049441670/src/Tgmpa/TgmpaServiceProvider.php#L31-L35:

if ( did_action( 'plugins_loaded' ) ) {
    load_tgm_plugin_activation();
} else {
    add_action( 'plugins_loaded', 'load_tgm_plugin_activation' );
}

with:

TGM_Plugin_Activation::get_instance();

which should work fine in combination with frameworks which autoload class files using Composer autoloading or another custom autoloading solution.

terence1990 commented 6 years ago

@jrfnl are you proposing a further check that should return early at the same point as ABSPATH check? In this case then the file wouldn't define the class TGM_Plugin_Activation which we need in our Themosis integration - so it can't return early. Or are you proposing to just move the did_action etc. code block higher up in the file?

I can use TGM_Plugin_Activation::get_instance(); in my service provider - thats fine, but the problem here is that the functions did_action and add_action are not defined at the point of composer autoload - so we need some way to condition this code to avoid a fatal error in Themosis.

terence1990 commented 6 years ago

i've decided to go down the route of PRS-4 convention in my fork, so will use this instead and try to keep it up to date. would be pleased to see PRS-4 in this project, take a look at my fork if you want any insight on that