- Issue created by @joegraduate
- πΊπΈUnited States joegraduate Arizona, USA
Updated module to decorate core config.installer service rather than alter it in MR !20. This made it easier to use the dynamic compatibility traits strategy employed by the config_selector module and made it possible to remove the proxy class. The proxy class was needed before because this module was extending the core ConfigInstaller class but that is no longer the case in MR.
- πΊπΈUnited States joegraduate Arizona, USA
Tested MR !20 with an existing project that contains a custom ConfigProvider plugin and found that the decorator approach used in that MR doesn't seem to work (the custom ConfigProvider plugin I was testing did not work as expected).
Created alternate MR !21 that adds a separate backwards-compatible `ConfigProviderConfigInstaller` class and updates the existing service provider to alter the core service using the appropriate `ConfigProviderConfigInstaller` class depending on the installed core version. The custom ConfigProvider in our existing project did still work as expected when tested with this MR.
- πΊπΈUnited States joegraduate Arizona, USA
joegraduate β changed the visibility of the branch 3532693-dynamic-alter to hidden.
- πΊπΈUnited States joegraduate Arizona, USA
Discussed the 2 different MR approaches extensively with fellow maintainer @tadean.
I've updated MR !20 with the approach that we both feel the most comfortable with which registers our
ConfigProviderConfigInstaller
class as a decorator for the coreconfig.installer
service via the services.yml file instead of altering (replacing) the core service via a ServiceProvider class. This approach vastly simplifies the MR because it:- removes the need for the existing ServiceProvider and Proxy classes,
- removes the need for the compatibility traits previously added in MR !20
- requires no changes to the existing
ConfigProviderConfigInstaller
service class
I've also hidden the other MR branch.
While discussing this issue and the potential fixes with @tadean, we realized that the changes proposed in @trackleft2's MRs to address π Address conflict with Features over config.installer class Active . We might want to consider closing this issue as a duplicate of that one and updating its IS with the 11.2 incompatibility details.
- πΊπΈUnited States joegraduate Arizona, USA
@tadean also noted that the removal of the ProxyClass and changing the lazy behavior of the config.installer service probably constitutes a performance regression (probably especially for sites using Drupal core < 11.2?).
It does seem like it would be desirable to keep the lazy behavior / ProxyClass if possible but the changes to the ConfigInstallerInterface introduced in 11.2 (the reason for this issue) make that a much more complicated task (because 2 different versions of the ProxyClass are needed to provide compatibility with Drupal >=11.2 and Drupal <11.2.
@joegraduate here's a patch versus MR !20 with a potential approach that keeps the lazy loading, but might be able to avoid the need for a backward compatibility Proxy class by inheriting from the
\Drupal\Core\ProxyClass\Config\ConfigInstaller
.Some considerations: this would be less code to maintain than the two-proxy approach (in MR !21), but perhaps there are reasons to avoid inheriting from the
\Drupal\Core\ProxyClass\Config\ConfigInstaller
? (Though, it is not marked internal). The approach in this patch makes no claims about the API of the Proxy by saying it's whatever the Core Proxy uses, essentially.Since all interface calls are deferred through the public functions in the lazy loading service via
lazyLoadItself()
, perhaps\Drupal\Core\ProxyClass\Config\ConfigInstaller
already does everything it needs to, we just need it be available under a different class name.