[Feature Request]: Fire an event when building with a generic hook name to be able to target it easily

[luislard]

Is your feature request related to a problem?

Currently, the only way to add modules from other places is via adding an action to this hook

The hook name is created dynamically based on the package name.

Since #44 was merged it would be very nice to have a way to add a module with an extension to all packages automatically.

So, in a project with many packages we could do:

add_action('inpsyde.modularity.package.init', fn($package) => $package->addModule($moduleWithExtensionByType));

Describe the desired solution

Maybe the solution is to add a hook.

Describe the alternatives that you have considered

Option 1:
Adding a generic hook

Option 2:
Using current hookName is also an option but you should hook in another hook like

function addExtensionCb (){
    /* some logic parsing current_action() name with startsWith inpsyde.modularity and endWith init */
}

add_action('muplugins_loaded', function(){
    add_action('all', 'addExtensionCb');
}, PHP_INT_MIN);

add_action('plugins_loaded', function(){
    remove_action('all', 'addExtensionCb');
}, PHP_INT_MAX)

Additional context

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[Chrico]

While I maybe agree with having a "generic" hookName to hook into all Packages, this could also introduce some side effects like hooking into a package which shouldn't be extended.

So instead of doing:

add_action(
	MyPackage\plugin()->hookName(Package::ACTION_INIT),
	static function(Package $plugin): void {
		$plugin->addModule( ... );
	}
);

you simply could loop over your packages by doing:

$packagesToExtend = [
	MyPackage\plugin()
];
foreach($packagesToExtend as $package) {
	add_action(
		$package->hookName(Package::ACTION_INIT),
		static function(Package $plugin): void {
			$plugin->addModule( ... );
		}
	);
}

[shvlv]

you simply could loop over your packages by doing:

The problem here is that you don't know whether MyPackage\plugin() exists. If the plugin is deactivated, you get fatal. This means we should check it with function_exists, which is pretty verbose.

[gmazzap]

I think the hook would be good to have.

Looping packages, besides using function_exists (which is also not the fastest function in town) has the problem that if you want to target all/most packages, then you might need to edit the function very frequently, e.g. every time a new plugin is added or removed.

Moreover, one issue of calling MyPackage\plugin() upfront, is that you might cause that plugin to boot earlier than expected. And if you "wait" using hooks, it might be too late.

To act as soon as the package is ready without side effects is impossible right now, because you need a package instance to add hooks, and obtaining that instance might have side effects.

To help avoid possible unwanted effects, we could fire the hook similarly to:

do_action(
   'inpsyde.modularity.init',
    $package->properties()->basename(),
    $package
);

This way, whoever listens to the event could easily filter on the basename. For example:

add_action(
   'inpsyde.modularity.init',
   static function ($basename, Package $package): void {
       if (str_start_with($basename, 'acme-'))  {
           $package->addModule(/* ...*/);
       }
   },
   10,
   2
);

This could also be used to build a flexible allow-list that you don't need to update often:

const PACKAGES_TO_EXTEND = [
  '^(inp)?syde-.+$',
  '^my-client-.+$',
  'something-else',
];

add_action(
   'inpsyde.modularity.init',
   static function ($basename, Package $package): void {
       foreach (PACKAGES_TO_EXTEND as $package) {
         if (preg_match("#$package#", $basename)) {
             $package->addModule(/* ...*/);
             return;
         }
       }
   },
   10,
   2
);

If a developer decides to do something with a Package instance without checking the package name... that's on them. I mean, we have to trust people to know what they are doing, and if they don't, there are so many different ways they can cause issues...

[gmazzap]

@Chrico it seems you agreed on having this hook.

Let's agree on :

• the name
• where exactly to fire it

I was thinking of adding this constant to the class:

public const ACTION_MODULARITY_INIT = self::HOOK_PREFIX . self::ACTION_INIT;

The constant evaluates to "inpsyde.modularity.init".

And then do:

do_action(
    self::ACTION_MODULARITY_INIT,
    $this->properties()->baseName(),
    $this
);

Immediately after the current package-specific "init" hook.

---

That said I was also thinking that we don't have any hook after the package is "built". In 1.7.0 we introduced built() but still if we want to hook at that point, which is the first point where it is safe to get a container, we can't do it.

The first point where it is safe to call $package->container() is here but there's no hook there.

If you call container() in the current "init" hook or in the new "generic" init hook, you get a fatal error. That is why I was thinking of adding this additional constant to the class:

public const ACTION_INITIALIZED = 'initialized';

And then, after the status is "initialized" do something like:

do_action(
    $this->hookName(self::ACTION_INITIALIZED),
    $this
);

I don't think we need a "generic" hook also there. But having a package-specific hook the first moment it is safe to call $package->container() I think it is useful.

What you (and everyone) think about this? If you agree, can we have a single PR that address both these two new hooks?

[Chrico]

I totally agree with the generic Package::ACTION_MODULARITY_INIT-hook. ✔️
But, by writing the text below, maybe the name is wrong. The status of the Package is STATUS_IDLE ( STATUS_INIT does not exists), so the action should reflect that as well? 😅

---

That said I was also thinking that we don't have any hook after the package is "built". In 1.7.0 we introduced built() but still if we want to hook at that point, which is the first point where it is safe to get a container, we can't do it.

I'm wondering what the use case for accessing the container via this hook at this time could be. Maybe we should revisit the workflow of registration, executing, resolving, extending and when which hook makes sense.

v1) We probably could even use the Package::progress() to trigger those hooks (we already have STATUS_READY and ACTION_READY) so we could have a mapping and automatically trigger those hooks. This could also have the downside that some context (like for "failure") is missing. So we need to ensure that all information is accessible through $this.

const STATUS_TO_ACTION = [
	self::STATUS_READY => self::ACTION_READY,
	self::STATUS_INITIALIZED => self::ACTION_INITIALIZED,
];

private function progress(int $status): void
{
	$action = self::STATUS_TO_ACTION[$status] ?? null;
	if($action !== null && !did_action($action)){
		do_action(
			$action, 
			$this->properties()->baseName(),
			$this
		);
	}

	$this->status = $status;
}

v2) Or we also could implement a similar logic as WordPress has with post_status changes where they have an action which contains "from-to":

self::HOOK_PREFIX . ".{$oldStatus}_to_{$newStatus}"

v3) Or - probably not that performant - we implement a

do_action( self::HOOK_PREFIX . '.progress', $oldStatus, $newStatus, $baseName, $this);

[gmazzap]

@Chrico I like the idea of emitting a hook on progress and I like the idea of having a map between statuses and hooks, even if we might need more statuses than hooks.

Statuses help us determine internally what happened and what is happening, while hooks open the class to the extern and we don't need to expose the class externally at any status change.

For the extern, we only need to expose:

1. a hook to allow external code to add services in the container (via modules or package connection)
2. a hook to allow external code to get staff in the container as soon as it is safe
3. a hook fired when the package is done executing all executable modules

So we have just 3 hooks but more statuses.

I think we could go with:

1. Package is constructed at STATUS_IDLE status
2. Modules are added (or package connected) directly by the code that instantiates the Package. No status change.
3. Build process starts. Status is changed to STATUS_INIT -- This would be new
4. ACTION_INIT hook is fired (package specific + global) -- The global hook would be new
5. Build process ends. Status is changed to STATUS_INITIALIZED
6. ACTION_INITIALIZED hook is fired. -- This would be new
7. Boot process starts. Status is changed to STATUS_BOOTING -- This would be new
8. Boot process ends. Status is changed to STATUS_READY
9. ACTION_READY hook is fired.
10. Status is changed to STATUS_DONE -- This would be renamed from "STATUS_BOOTED"

This could be done in a 100% backward compatible way.

So we would have statuses like:

public const STATUS_IDLE = 2;
public const STATUS_INIT = 3;
public const STATUS_INITIALIZED = 4;
public const STATUS_BOOTING = 5;
public const STATUS_READY = 7;
public const STATUS_DONE = 8;

And a "map" like:

private const STATUSES_ACTIONS_MAP = [
    self::STATUS_INIT => self::ACTION_INIT,
    self::STATUS_INITIALIZED => self::ACTION_INITIALIZED,
    self::STATUS_READY => self::ACTION_READY,
];

There's IMO no need for an action for self::STATUS_BOOTING because, at that point, the container is already locked, so it can be used to add services, and to access services there're already two hooks, one before and one after, so I don't think this additional hook would be useful at all.

If you agree with the above I can send a PR.