diff --git a/docs/actions.md b/docs/actions.md index a131251..fad31b1 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -5,14 +5,43 @@ you override. `{prefix}` is the value passed to `Config::set_hook_prefix()`. | Action | Arguments | Fires when | |---|---|---| +| `{prefix}/plugin_absorber/loading` | `Sub_Plugin $sub_plugin` | Every gate has passed and the bundled file is about to be required. | | `{prefix}/plugin_absorber/loaded` | `Sub_Plugin $sub_plugin` | A bundled file was required, and its activation callback has already run. | | `{prefix}/plugin_absorber/skipped` | `Sub_Plugin $sub_plugin`, `string $reason` | A gate turned a sub-plugin away. | -Between them they cover what the load pass does with a sub-plugin it reached: one that loaded, and -one a gate turned away. They are not a census of what you registered — see below for what neither -of them announces. +Each is accurate for the sub-plugin it names, but some registered sub-plugins announce nothing at +all, so `loaded` and `skipped` together do not add up to everything registered: -## Loading +- A sub-plugin whose `enabled`, `dependency_check` or `should_load` callable throws announces + nothing. +- One whose bundled file throws as it is required has announced `loading`, but announces neither + `loaded` nor `skipped`. +- A `DEACTIVATE` conflict that deactivates a standalone copy redirects before the load pass runs, so + nothing is announced on that request. + +## Loading, before the require + +`loading` is for what has to be in place *before* the bundled file runs — registering an autoloader +for the namespace it ships is the usual reason, since the file may reference its own classes at file +scope: + +```php +add_action( 'give/plugin_absorber/loading', function ( $sub_plugin ) { + if ( $sub_plugin->get_slug() === 'give-recurring' ) { + My_Autoloader::register( 'Give\\Recurring\\', __DIR__ . '/sub-plugins/recurring/src' ); + } +} ); +``` + +Do not reach for the [`should_load` filter](filters.md#the-load-gate) instead. `Conflict\Detector` +applies it a priority earlier to decide whether a standalone copy is in the way, so a listener there +also fires in the case where this copy is about to be turned away — the opposite of what you wanted. + +Unlike the two below, a listener that throws here is **not** caught by the announcement. The require +has not happened, so the throw falls to the load pass, which abandons that sub-plugin and reports it. +A host that could not prepare gets no bundled copy rather than a half-ready one. + +## Loaded `loaded` is the answer to "is this sub-plugin here?" without a `defined()` check of your own, and it is where code that builds on a sub-plugin belongs: @@ -53,17 +82,13 @@ add_action( 'give/plugin_absorber/skipped', function ( $sub_plugin, $reason ) { The values are fixed API and will not change. New reasons may be added, so treat one you do not recognise as a plain skip rather than as an error. -`loaded` and `skipped` do not add up to everything registered, so do not count on them to. A -sub-plugin whose `enabled`, `dependency_check` or `should_load` callable throws, or whose bundled -file throws as it is required, announces neither; and a `DEACTIVATE` conflict redirects before the -load pass runs at all, so on that request no sub-plugin announces anything. - ## Your listener cannot take the site down -These fire from inside `plugins_loaded`, so a listener that throws is caught rather than allowed -out. It costs nothing: by the time `loaded` fires the require has happened, the guard constant has -been checked and the activation callback has run, and a `skipped` announcement is the last thing that -happens to that sub-plugin either way. The throw is reported through `_doing_it_wrong()` as what it -is — a listener, named by the hook it is on — rather than as the sub-plugin having failed, so a host -reading its log does not mistake its own bug for a load that broke. That is a backstop, not a -licence — a listener here runs on every request the site serves, so keep it cheap and keep it quiet. +`loaded` and `skipped` fire from inside `plugins_loaded`, so a listener that throws is caught rather +than allowed out. (`loading` is the exception, for the reason given above.) It costs nothing: by the +time `loaded` fires the require has happened, the guard constant has been checked and the activation +callback has run, and a `skipped` announcement is the last thing that happens to that sub-plugin +either way. The throw is reported through `_doing_it_wrong()` as what it is — a listener, named by +the hook it is on — rather than as the sub-plugin having failed, so a host reading its log does not +mistake its own bug for a load that broke. That is a backstop, not a licence — a listener here runs +on every request the site serves, so keep it cheap and keep it quiet. diff --git a/src/Loader.php b/src/Loader.php index da9899d..a9c937e 100644 --- a/src/Loader.php +++ b/src/Loader.php @@ -181,6 +181,15 @@ private function load( Sub_Plugin $sub_plugin ): void { return; } + // The last point before the require, and the only one where a host can put something in place + // that the bundled file needs at its own file scope. `should_load` is not that point: + // `Conflict\Detector` applies the same filter a priority earlier, when a standalone copy is + // in the way and this one is about to be turned away. + // + // Not through announce(): that swallows a listener's throw, which is wrong with the require + // still ahead. `load_all()` catches it and abandons the sub-plugin, which is accurate here. + do_action_ref_array( Config::get_hook_name( 'loading' ), [ $sub_plugin ] ); + // An include takes the scope of the line it sits on, so top-level assignments in the bundled // file are function-local where wp-settings.php would have made them global. Not fixable. require_once $file; diff --git a/tests/unit/LoaderTest.php b/tests/unit/LoaderTest.php index c635048..60b3e59 100644 --- a/tests/unit/LoaderTest.php +++ b/tests/unit/LoaderTest.php @@ -406,6 +406,125 @@ public function test_an_already_loaded_sub_plugin_is_not_dependency_checked(): v $this->assertSame( 1, $checked, 'The recorder must catch a call that really happened.' ); } + /** + * Tests that the announcement lands with the require still ahead of it. + * + * The whole point of the hook: a listener registers what the bundled file needs at its own file + * scope, so it has to run first. + * + * @return void + */ + public function test_the_loading_action_fires_before_the_require(): void { + $this->register(); + + $loads_at_announcement = null; + add_action( + 'give/plugin_absorber/loading', + function () use ( &$loads_at_announcement ) { + $loads_at_announcement = $this->bundled_plugin_loads(); + } + ); + + $this->loader()->load_all(); + + $this->assertSame( 0, $loads_at_announcement, 'The require must still be ahead of the listener.' ); + $this->assertSame( 1, $this->bundled_plugin_loads() ); + } + + /** + * Tests that a listener is handed the sub-plugin about to be loaded. + * + * @return void + */ + public function test_the_loading_action_receives_the_sub_plugin(): void { + $this->register(); + + $received = null; + add_action( + 'give/plugin_absorber/loading', + static function ( $sub_plugin ) use ( &$received ) { + $received = $sub_plugin; + } + ); + + $this->loader()->load_all(); + + $this->assertInstanceOf( Sub_Plugin::class, $received ); + $this->assertSame( 'give-recurring', $received->get_slug() ); + } + + /** + * Tests that a sub-plugin a gate turned away announces nothing. + * + * @return void + */ + public function test_the_loading_action_does_not_fire_for_a_sub_plugin_a_gate_turned_away(): void { + $this->register( [ 'enabled' => false ] ); + + $fired = false; + add_action( + 'give/plugin_absorber/loading', + static function () use ( &$fired ) { + $fired = true; + } + ); + + $this->loader()->load_all(); + + $this->assertFalse( $fired ); + $this->assertSame( 0, $this->bundled_plugin_loads() ); + } + + /** + * Tests that a veto on the last gate stops the announcement as well as the require. + * + * @return void + */ + public function test_the_loading_action_does_not_fire_when_the_should_load_filter_vetoes(): void { + $this->register(); + + add_filter( 'give/plugin_absorber/should_load', '__return_false' ); + + $fired = false; + add_action( + 'give/plugin_absorber/loading', + static function () use ( &$fired ) { + $fired = true; + } + ); + + $this->loader()->load_all(); + + $this->assertFalse( $fired ); + } + + /** + * Tests that a throwing listener stops the require rather than being swallowed. + * + * `announce()` catches a listener's throw, which is right once the require has happened and wrong + * before it. This one falls to `load_all()`, which abandons the sub-plugin. + * + * @return void + */ + public function test_a_throwing_loading_listener_abandons_the_sub_plugin_before_the_require(): void { + $this->register(); + + add_action( + 'give/plugin_absorber/loading', + static function (): void { + throw new RuntimeException( 'the host could not prepare' ); + } + ); + + $this->expect_incorrect_usage(); + + $this->loader()->load_all(); + + // Abandoned rather than half-loaded: announce() would have swallowed this and required the + // file anyway. + $this->assertSame( 0, $this->bundled_plugin_loads() ); + } + public function test_the_should_load_filter_can_veto_the_load(): void { $this->register();