Ensure all modules are autoloaded with PSR-4 only if enabled

Time has passed, new perspectives and such.

Following on a suggestion and discussion @ #3522410-11: Using instances of classes defined by modules in service container parameters causes fatal errors →

What if we did the inverse to what is currently proposed here, back to the original intent of the ticket before direction change in #2536610-6: Ensure all modules are autoloaded with PSR-4 only if enabled →

Always have all modules available in autoloader.

Then, the concept of "installed" means whether a modules hooks/services/discovery/plugins etc etc is loaded.

Always have all modules available in autoloader.

+1 from me as a contrib developer.

Yes as #6 calls out this will require contrib to do some work with the (hacky) use of class_exists() that has been used to determine if a module was enabled, however we now have a robust system with Drupal Rector that would allow automated updates by converting class_exists('\Drupal\some_module\some_class') to \Drupal::service('module_handler')->moduleExists('some_module') && class_exists('\Drupal\some_module\some_class')

For DX: I have encountered this in a couple scenarios related to implementing interfaces:

Scenario 1:
Decorating a core service that may not always be enabled using autowire:
The decorator has to implement the service interface, however with the core module namespace not resolvable and the design of Symfony PHP will throw a fatal error that the interface was not found. If the interface was resolvable Symfony onInvalid: ContainerInterface::IGNORE_ON_INVALID_REFERENCE should allow for the service to remain autowired yet not registered in the container.

Scenario 2 (I'm working through this with contrib modules right now)
Plugin implementing interfaces from two modules, where one is optional additional features:
Module A exists in contrib and provides a plugin system.
Module B wishes to extend the features of module A, to do this it will provide an additional interface that third party plugins will implement that provides these features.
Module C implements a Module A plugin and also wishes to implement the enhancements of Module B.

In this scenario Module B must be enabled for Module C's plugins to load Module B's interface forcing the site to use Module B even if they do not desire the extra features. As above this would be due to PHP throwing a fatal error for an unknown interface. If Module B's namespace were resolvable it need only be a composer dependency not an install dependency.

Of course there are ways around both of these, such as with use of additional meta packages, custom ServiceProvider alters, not using autowiring, etc, however these push us away from aligning with upstream support and the general PHP ecosystem.

Scenario #2 from #30 should be solved by providing two plugin classes, the second one implementing the interface from module C. If you wanted this to be automatically used by existing config specifying the first plugin, then the class could be swapped out by a plugin info alter.

Having a composer dependency on a module that may or may not be installed can introduce problems if module C isn't compatible with newer Drupal core versions - the site won't be able to update, and it also won't be able to composer remove because that would lead to the fatal error again.

Having a composer dependency on a module that may or may not be installed can introduce problems if module C isn't compatible with newer Drupal core versions -

For clarity, I believe you mean Module B.

That really is no different than having a dependency on module that can be installed that isn't compatible (you can't PMU it if its a Drupal Dependency of Module C) or a dependency on any other composer library that would conflict with Drupal Core.

If you choose to implement a dependency or even install a package on your Drupal site you need to have vetted the developer to be sure they will continue to maintain the software (or be prepared to dump them should it be required).

should be solved by ...

For brevity I did not list all the scenarios that had been considered and failed an initial architectural review.

If you choose to implement a dependency or even install a package on your Drupal site you need to have vetted the developer to be sure they will continue to maintain the software (or be prepared to dump them should it be required).

No it's not the same, because you're specifically talking about the case of a contrib developer making a deliberate choice to add a hard composer dependency on another contrib module, instead of making that dependency properly optional. If the dependency is genuinely optional, then version compatibility issues are easier to mitigate later. Saying 'it's the site owner's fault if they install a module' is not really applicable in the context of an issue against core specifically to make bad scenarios as described above more likely.

No it's not the same, because you're specifically talking about the case of a contrib developer making a deliberate choice to add a hard composer dependency on another contrib module, instead of making that dependency properly optional.

Welcome to the PHP Language. Interfaces will/must be loaded by the parser to be implemented. This is no different than being dependent upon psr/log for the interfaces it provides.

Loading the namespace into the autolaoder makes the module optional, otherwise (one of) the (possible) solution is that Module C has to make Module B (the module that provides the interface) a hard dependency. Autoloading makes it so as long as the interface is properly formatted PHP it loads (Module B would then have config options to disable its features, but now it sits taking up space and processing time in all Drupal related module functions such as the hook dispatchers, updates checker, etc)

it's the site owner's fault if they install a module

To be clear, the primary fault was directed to the developer who failed to vet their dependencies. The site owner example was thrown in as secondary responsibility.

Welcome to the PHP Language. Interfaces will/must be loaded by the parser to be implemented. This is no different than being dependent upon psr/log for the interfaces it provides.

The plugin system includes explicit support for discarding plugins that implement missing interfaces during discovery to support use cases like the one above - e.g. modules can ship plugins that will be discovered only when the module the plugin requires is installed. You're pretending as if this doesn't exist and no-one has thought about this before.

Loading the namespace into the autolaoder makes the module optional

It doesn't make it optional in the code base you're arguing for the exact opposite here.

There's also an existing use case now that #30 would break.

Module A implements e.g. a field formatter plugin that should only be discovered when module B is installed, because it relies on features in in module B. Let's say it depends on a formatter base clase or trait that module B provides.

With HEAD, the field formatter that module A provides will only be discovered by the plugin system when module B is installed.

With the change here, it would be discovered whenever module B is a composer dependency. However, the base class or trait may rely on other code in module B other than the trait/base class - so actually trying to use it when module B isn't installed could result in fatal errors or it otherwise not working at all.

It also means module A would behave differently in three ways rather than two - when module B is installed, when it's required by composer but not installed, and when it's not required by composer at all.

There is no proposal here for how to provide backwards compatibility for that case or otherwise how it would be addressed.

it doesn't make it optional in the code base you're arguing for the exact opposite here.

Sorry if there is confusion around this point. I have been trying to say that yes the composer package is not optional in the code base, the Drupal Module is optional from the standpoint of Drupal core operations. (Module disabled, Drupal doesn't care that it is on disk, composer however can still access the libraries/interfaces).

There is no proposal here for how to provide backwards compatibility for that case or otherwise how it would be addressed.

Thank you for providing a technical concern that is rooted in a current logic instead of developer design choice. I don’t have an answer for that scenario at this moment and would need to take time to consider the impact.

I do wonder if this issue been acted upon sooner, or at least been in the roadmap for all new designs, might we have avoided those concerns as part of the attributes conversion. We can't change the past, and just have to push on, just want to annotate how we could possible work on additional coordination during design phases.

We had to workaround a PHP bug with missing traits to support attribute discovery in

Indeed. I believe I was monitoring a number of issues related to annotations conversions including that issue.

interfaces it provides into a separate 'contracts' component

FWIW that’s currently the leading contender for my project at the moment, though option #30-2 would IMO having been through the design evaluation also be significantly valid in this particular case.

Realistically Module B belongs inside Module A’s code base, however when maintainers don't act we look at the options.

You're pretending as if this doesn't exist and no-one has thought about this before.

There is significantly more in my designs evaluations than can be laid out here in short blurbs, especially since the majority is not directly related to this issue. I have noted previously some ideas you have suggested would work technically they failed the DX review which leads to the scenario proposed.

@cmlara can you share your full design concerns somewhere so we can evaluate?

Whether it affects the likelihood of this issue being resolved I cannot say for now but I think it would be informative to read what seems like a deep analysis as I consider issues in general.