Reconsider structural requirements

[Biont]

Concerns

I would really like you to reconsider the amount of structural conventions surrounding the use of ModuleInterface. As you know, I have spent some time toying with adding IDE support to modular projects via a composer plugin and one of the things I couldn't quite solve is dealing with constructor arguments. (->The plugin currently only inspects the bare implementations of ModuleInterface without respecting the loading pattern via module.php.

So the natural thing would be to extend support to the broader spec laid out in the README, but I cannot stop complaining. Hence, this issue.

It feels to me as if this whole module.php pattern was mostly born out of the inability to safely bootstrap a collection of modules based on the composer package type alone¹ I think the beauty of the interface itself is that it's just 2 methods + the dependency on service-provider. But here you are blowing it up with a large amount of outside requirements:

• the presence of a module.php file
• a specific type of package
• a mechanism to install the package type to a specified folder

Technically, only the module.php file is spelled out as a hard requirement that a module MUST fulfill, while the rest might serve as an "example implementation" without being declared as such.

But module.php is such a weird (and weirdly specific) requirement to make, and it got criticised before: https://github.com/Dhii/module-interface/pull/16#pullrequestreview-375438320

It's also somewhat contradicted by the existence of ModuleAwareInterface. Why add an interface that would allow modules to be retrieved from a class instance if you set in stone that it must be provided by module.php. I don't think it makes sense to define two opposing approaches to retrieve a module instance here.

Another thing that bothers me is that - while it would absolutely be possible to support module.php in the IDE helper- you still cannot be sure you can instantiate a module without having to satisfy dependencies that a composer plugin cannot provide.

In other words: I think the goal of requiring a module.php file was to enforce a standard way of supplying module factories in order to steer around the requirements of any individual module's constructor arguments. But it does not and cannot enforce the factory function itself to be devoid of function parameters so this point falls flat a bit. I think requiring a factory at all is a pretty hefty part of the spec when this should ideally be each individual application's concern. And if you're gonna go there, I would really like to see a more flexible approach - ideally one that allows usage of the standard without a factory as well

Suggestions

I've been thinking of two things:

1. Changing ModuleAwareInterface so that getModule is static²:

   /**
   * Something that can have a module instance retrieved.
   *
   * @since 0.2
   */
   interface ModuleAwareInterface
   {
   /**
    * Retrieves the module that is associated with this instance.
    *
    * @since 0.2
    *
    * @return ModuleInterface|null The module, if applicable; otherwise, null.
    */
   public static function getModule();
   }

2. Instead of requiring a module.php as part of the standard, suggest that implementing packages SHOULD contain an entry in the composer.json like this:

   {
   "extra": {
       "dhii": {
           "module-factory": "MyModuleProvider"
       },
   }
   }

   "module-factory" could be one of the three:

3. The FQCN of a class implementing ModuleAwareInterface which can now be called statically (erasing the dependency problem in the safest manner)

4. The name of a function that returns a ModuleInterface

5. The filename of a php file containing/returning a function that provides a ModuleInterface

6. Absent - in which case we could still look at the implementation(s) of ModuleInterface themselves to see if they can be instantiated without constructor arguments. That, or search the package for any ModuleAwareInterface² and use that. In these scenarios, specifying this composer config would practically serve just as an override if the default behaviour is undesirable. Another reason why it would make sense to make this an optional requirement to begin with

Doing this would allow trimming down the spec to its bare minimum again while still fully satisfying any existing workflows. AND you'd introduce a standardized way for composer plugins to integrate with the module packages. And this doesn't even require them to be of a specific package type any longer.

---

¹This is something the composer plugin could really help out with by simply dumping all modules found. Of course, this gets more complex as soon as you need to customize the load order, but maybe a solution can be found for that as well

² Maybe this should be renamed to ModuleProvider or put into a new class after all. I have not thought too much about the anticipated use-cases of ModuleAwareInterface and just more or less assumed it's currently just sitting there unused by anyone

[mecha]

This, to me, seems to summarize the entire issue:

Maybe this should be renamed to ModuleProvider or put into a new class after all. I have not thought too much about the anticipated use-cases of ModuleAwareInterface and just more or less assumed it's currently just sitting there unused by anyone

Firstly, the "Provider" naming scheme is something I'm tending more towards recently, so I'm all for it. It's more meaningful and the same terminology is also used in other languages.

Secondly, I don't believe there is any actual need for ModuleAwareInterface. Note that I'm not saying that no one will need an object that provides a module; there could very well be a use case for such an object. What I'm saying is that there's probably no need for a standard interface on which a set of module-related systems rely, not when modules can be simply instantiated by the system.

Module aggregation is already a very app-specific thing just by its very nature: the app installs its preferred module aggregator (such as a custom composer installer) and then uses it to obtain a list of modules. The ModuleAwareInterface doesn't come into play here either.

Though, I would go a step further and say that a module aggregator isn't even necessary at all. As you put it, the beauty of the standard is that it's just an interface, and implementers are just a class. Meaning that this alternative to module aggregation and bootstrapping works just as well:

<?php // modules.php

return [
    'core' => new CoreModule(),
    'ui' => new UiModule(),
    'db' => new DbModule(),
    'migration' => new MigrationModule(),
    'logging' => new LoggingModule(),
    'wp_cron' => new WpCronModule(),
    'rest_api' => new RestApiModule(),
];

Just as how composer.json lists dependencies by name, here too the application lists its modules by name, no abstraction required. There's also the added benefit of being able to manipulate this list in an infinitely larger number of ways. In our case at work, this list is statically manipulated by our deployment tool to produce different tiers of the product from different sets of modules.

And that's sort of the entire point. The above approach would be considered non-standard by the spec, but I believe the spec shouldn't have a say in this.

So I agree; the only truly important part of this package is ModuleInterface, the interface that all modules implement thus ensuring that all modules are compatible with a modular application. Anything else provided by this package isn't really useful (in my humble opinion).

[XedinUnknown]

I agree that ModuleAwareInterface isn't really used right now. But it could be: if something like a PackageInterface had to expose a module, ModuleAwareInterface would be the interop way of doing that. But I am open to removing it, because if there is a need later on, then we can analyze the requirements and come up with an appropriate solution based on usecases, and add a new interface here without breaking BC at any time.

About module.php. This is a way to standardize module constructors. But it isn't really part of the spec of the standard. It is only a way of going about the problem. The reason it is in the docs is that people have requested documentation quite a lot. @mecha had pointed out to me that these docs shouldn't really live in this repository, but at the time for the lack of a more centralized location I added it here. Nowadays there's an example in wp-oop/plugin-boilerplate, and I guess a lot of the extra conventions can be removed from this package. The module.php convention can be used in certain applications, but it really doesn't have to be - because as @mecha again points out, the way the application obtains its ModuleInterface instances is up to it, different modules may have different ways of discovery, and in the end it's just a map/list of modules. The signature of module.php is also just as much particular to the "framework" that could be using modules. There are opportunities for autodiscovery too, and although I don't encourage it, it's a possible scenario.

I would consider adding support for discovery via Composer to something like plugin-boilerplate, but again this isn't really a standard for the module-interface package. One thing I would be really hesitant about, though, is static methods. In any case, a simple map or list is enough right now, and even if invoking module constructors directly, the application should be able to know how to satisfy module dependencies - and be able to satisfy them. If it isn't able to satisfy module dependencies, then it cannot use the module in any case - whether or not the constructor is abstracted.

@Biont, do you require the Composer config in order to be able to add e.g. IDE support?

So, I would say for now: remove ModuleAwareInterface, and write a clear minimal example of how to load a bunch of modules - like in these bootstrap instructions.

Thoughts?

[Biont]

do you require the Composer config in order to be able to add e.g. IDE support?

No, I simply iterate over any found implementations of ModuleInterface that can be found under the source root.

And as I said, the composer config might as well be...

Absent - in which case we could still look at the implementation(s) of ModuleInterface themselves to see if they can be instantiated without constructor arguments. That, or search the package for any ModuleAwareInterface² and use that. In these scenarios, specifying this composer config would practically serve just as an override if the default behaviour is undesirable. Another reason why it would make sense to make this an optional requirement to begin with

This is really something optional. But if it was suggested (not required) by the standard, it would serve as a great base for those sorts of applications that opt to use composer plugins for discovery/IDE-integration

---

The module.php convention can be used in certain applications, but it really doesn't have to be

Well this is not really true and it's the entire reason I've opened this issue:

In your module's pacakge, create a file that returns a module factory. This factory MUST return an instance of ModuleInterface from this pacakge.

(emphasis mine)

And this is the very first actual sentence of the usage documentation. The use of an uppercase "MUST" (like most standard docs I'm aware of) strongly suggests that "If you don't do that, you breaking the spec".

At the end of the day, all I really want is a clear separation between standard definition and suggested usage example. And I want to mention that while I'm sitting here complaining, I really appreciate the documentation/examples

[XedinUnknown]

Thanks, @Biont! I apprecate your contribution and involvement!

Yes, I'm all for that separation. To be precise, the "MUST" refers to the module.php having to return an instance of ModuleInterface, not to having to have a module.php. But again, this is only a spec of an example system, as has been pointed out by all of us. I would very much like to separate that.

I think that the best way to go about such separation is to create an actual modular framework that allows quick application building using the modular system, and other Dhii and non-Dhii standards and implementations. Maybe, something like wp-oop/plugin-boilerplate, but not for WP.

How does that sound?

[XedinUnknown]

In fact, I had already created a repo where I try to retrospectively remove the WP aspects of wp-oop/plugin-boilerplate, and standardize where possible the approach used in those two plus dhii/php-template. Do you think we can contibue there?