Don't pass the $plugins argument of the onPluginsLoaded event by reference

This is a BC breaking change!

Manipulating Pico's $plugins array is a really bad idea. We've introduced the Pico::loadPlugin() method to safely load plugins at any time, however, Pico might do unexpected things when loading plugins too late. See the class docs of Pico::loadPlugin() for more details. Nevertheless, this change breaks BC to Pico 1.0. However, I don't know a single plugin that relies on manipulating the $plugins array. If you just want to load a plugin manually, use Pico::loadPlugin() instead.
This commit is contained in:
Daniel Rudolf 2017-05-13 18:17:19 +02:00
parent 6e28a51080
commit 191f6edbe9
No known key found for this signature in database
GPG Key ID: A061F02CD8DE4538
2 changed files with 37 additions and 4 deletions

View File

@ -363,7 +363,7 @@ class Pico
// load plugins // load plugins
$this->loadPlugins(); $this->loadPlugins();
$this->sortPlugins(); $this->sortPlugins();
$this->triggerEvent('onPluginsLoaded', array(&$this->plugins)); $this->triggerEvent('onPluginsLoaded', array($this->plugins));
// load config // load config
$this->loadConfig(); $this->loadConfig();
@ -548,7 +548,23 @@ class Pico
/** /**
* Manually loads a plugin * Manually loads a plugin
* *
* Manually loaded plugins must implement {@see PicoPluginInterface}. * Manually loaded plugins MUST implement {@see PicoPluginInterface}. They
* are simply appended to the plugins array without any additional checks,
* so you might get unexpected results, depending on *when* you're loading
* a plugin. You SHOULD NOT load plugins after a event has been triggered
* by Pico. In-depth knowledge of Pico's inner workings is strongly advised
* otherwise, and you MUST NOT rely on {@see PicoDeprecated} to maintain
* backward compatibility in such cases.
*
* If you e.g. load a plugin after the `onPluginsLoaded` event, Pico
* doesn't guarantee the plugin's order ({@see Pico::sortPlugins()}).
* Already triggered events won't get triggered on the manually loaded
* plugin. Thus you SHOULD load plugins either before {@see Pico::run()}
* is called, or via the constructor of another plugin (i.e. the plugin's
* `__construct()` method; plugins are instanced in
* {@see Pico::loadPlugins()}).
*
* This method triggers the `onPluginManuallyLoaded` event.
* *
* @see Pico::loadPlugins() * @see Pico::loadPlugins()
* @see Pico::getPlugin() * @see Pico::getPlugin()
@ -584,6 +600,10 @@ class Pico
$this->nativePlugins[$className] = $plugin; $this->nativePlugins[$className] = $plugin;
} }
// trigger onPluginManuallyLoaded event
// the event is also called on the newly loaded plugin, allowing you to distinguish manual and auto loading
$this->triggerEvent('onPluginManuallyLoaded', array($this->plugins[$className]));
return $plugin; return $plugin;
} }

View File

@ -50,10 +50,23 @@ class DummyPlugin extends AbstractPicoPlugin
* *
* @see Pico::getPlugin() * @see Pico::getPlugin()
* @see Pico::getPlugins() * @see Pico::getPlugins()
* @param object[] &$plugins loaded plugin instances * @param object[] $plugins loaded plugin instances
* @return void * @return void
*/ */
public function onPluginsLoaded(array &$plugins) public function onPluginsLoaded(array $plugins)
{
// your code
}
/**
* Triggered when Pico manually loads a plugin
*
* @see Pico::getPlugin()
* @see Pico::getPlugins()
* @param object $plugin loaded plugin instance
* @return void
*/
public function onPluginManuallyLoaded($plugin)
{ {
// your code // your code
} }