From e2f4d971567a35fd3bb653906b56c7ecfe3f72c6 Mon Sep 17 00:00:00 2001 From: Eric Defore Date: Thu, 27 Aug 2026 20:29:05 -0400 Subject: [PATCH 1/3] Announce a sub-plugin before its file is required The load pass had no seam between its last gate and the require, so a host with work to do before the bundled file runs -- registering an autoloader for the namespace it ships, most often, since that file may reference its own classes at file scope -- had nowhere to put it. should_load is not that seam, though it looks like one. Conflict\Detector applies the same filter a priority earlier to decide whether a standalone copy is in the way, so a listener there also fires in the case where the bundled copy is about to be turned away. Fired directly rather than through announce(). A listener that throws on this side of the require should not be swallowed: load_all() catches it and abandons the sub-plugin, which leaves the host with no bundled copy rather than a half-ready one. --- docs/actions.md | 38 +++++++++--- src/Loader.php | 9 +++ tests/unit/LoaderTest.php | 119 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 7 deletions(-) diff --git a/docs/actions.md b/docs/actions.md index a131251..179f0cd 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -5,14 +5,37 @@ 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. +Between them they cover what the load pass does with a sub-plugin it reached: one that is about to +load, one that loaded, and one a gate turned away. They are not a census of what you registered — +see below for what none of them announces. -## Loading +## 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,15 +76,16 @@ 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 +`loading`, `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 +`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 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(); From 323bffcb9077bf430417fae45b105bc81e30f7aa Mon Sep 17 00:00:00 2001 From: Eric Defore Date: Thu, 27 Aug 2026 20:43:15 -0400 Subject: [PATCH 2/3] Correct what the actions cover now that loading fires first A bundled file that throws as it is required has already announced loading, so the paragraph saying it announces neither was true only before this hook existed. The sentence had also been left unfinished. --- docs/actions.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/actions.md b/docs/actions.md index 179f0cd..2e1c485 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -76,11 +76,11 @@ 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. -`loading`, `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. +These three do not add up to everything registered, so do not read them as a census. A sub-plugin +whose `enabled`, `dependency_check` or `should_load` callable throws announces none of them. One +whose bundled file throws as it is required has already announced `loading`, and will announce +neither `loaded` nor `skipped`. 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 From 86cc13114288df5bbebdb89f7064ed34605e2330 Mon Sep 17 00:00:00 2001 From: Eric Defore Date: Thu, 27 Aug 2026 20:57:42 -0400 Subject: [PATCH 3/3] Simplify what the actions do and do not announce --- docs/actions.md | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/docs/actions.md b/docs/actions.md index 2e1c485..fad31b1 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -9,9 +9,15 @@ you override. `{prefix}` is the value passed to `Config::set_hook_prefix()`. | `{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 is about to -load, one that loaded, and one a gate turned away. They are not a census of what you registered — -see below for what none 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: + +- 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 @@ -76,18 +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. -These three do not add up to everything registered, so do not read them as a census. A sub-plugin -whose `enabled`, `dependency_check` or `should_load` callable throws announces none of them. One -whose bundled file throws as it is required has already announced `loading`, and will announce -neither `loaded` nor `skipped`. 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 `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. +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.