Migrate DI container from Dhii to inpsyde/modularity (510)#9
Migrate DI container from Dhii to inpsyde/modularity (510)#9danieldudzic wants to merge 28 commits intodev/mainfrom
Conversation
composer.lockPackage changes
Dev Package changes
Settings · Docs · Powered by Private Packagist |
|
|
||
| ```bash | ||
| ddev composer require --dev coenjacobs/mozart:^0.7.1 -W | ||
| ddev exec vendor/bin/mozart compose |
There was a problem hiding this comment.
I did not look into everything else yet, but we probably do not need to use Mozart here. In PCP we used it because PHP-Scoper had some issues at that time. Now we have PHP-Scoper during build, so it should be fine to just use it for everything.
There was a problem hiding this comment.
@AlexP11223 I looked into removing Mozart, but it breaks local dev.
When testing with a plugin that bundles a PSR container (like wc-smooth-generator bundles psr/container 1.x), its autoloader takes precedence over ours, so the untyped PSR-11 1.x ContainerInterface gets registered. Then inpsyde/modularity (which uses typed PSR-11 2.x signatures) throws a fatal error.
There was a problem hiding this comment.
hm, yeah, if we need to use such plugins, then I guess it's the only way.
Though would be great to nudge their developers to update. :)
There was a problem hiding this comment.
How common is this during development? AFAIK, PoS would be the only project that requires this workaround during development.
Refactoring our entire codebase against scoped symbols is something we decidedly wanted to get away from when committing to php-scoper during build.
For testing compatibility issues, it is very easy to throw the offending plugins and a build from GHA into a quick wp-env setup and debug there.
Since this repo here is public, we could even point wp-env directly to a build branch.
I really would not use mozart here. It's an extremely invasive change, a constant cat & mouse game and hard to integrate with libraries. And we already have php-scoper for free.
How big is the pain with wc-smooth-generator?
There was a problem hiding this comment.
Maybe one workaround for this generator plugin could be to simply disable our plugin when generating something, I guess it would not need to be done often.
But, yes, as of right now, I think this plugin would not be super useful. We only work with products, and currently the main things that matter are product type and the inventory number.
Usually we were just using the WooCommerce sample products, which iirc are created automatically in DDEV here.
There was a problem hiding this comment.
@Biont Actually, PayPal Payments already uses exactly this workaround. The composer.json configures Mozart to rewrite both psr/container and inpsyde/modularity into the WooCommerce\PayPalCommerce\Vendor\ namespace:
"mozart": {
"dep_namespace": "WooCommerce\\PayPalCommerce\\Vendor\\",
"dep_directory": "/lib/packages/",
"classmap_directory": "/lib/classes/",
"classmap_prefix": "PCPP_",
"packages": [
"psr/container",
"inpsyde/modularity"
],
"delete_vendor_directories": true
}So the production code uses the Mozart-prefixed copy (WooCommerce\PayPalCommerce\Vendor\Psr\Container\ContainerInterface), and psr/container is only in require-dev — it's not a production dependency.
There's a bunch of plugins that utilize the PSR container these days, and we shouldn't be limited (if possible) by our own setup regarding what we can test locally or have to alter our flows. I fundamentally want the same freedom while developing locally as when testing post-production.
There was a problem hiding this comment.
@danieldudzic Pardon my ignorance then.
However, I would like to hazard a guess that this is most likely an artifact from a time before build & distribute and php-scoper were introduced.
- No Composer scripts trigger it — the scripts section only defines ci (phpcs) and unit (phpunit). Mozart's typical hook would be post-install-cmd or post-update-cmd, and
neither is present. - No allow-plugins entry for Mozart — the config.allow-plugins block doesn't include coenjacobs/mozart, so even if it tried to act as a Composer plugin, it would be blocked.
- Our packaging workflows don't execute it
To run it, you'd need to invoke it manually: vendor/bin/mozart compose.
The workaround is only used, because the code is already committed to VCS and now the convention is sticking around. The damage is done and noone went back in and fixed the mess after we introduced php-scoper. The mozart config should be removed and the code unscoped - but that's a different discussion.
Here in PoS we still have the option to prevent bending our source codebase around potential issues with random third-party plugins - issues which will be gone in production because we have full scoping anyway
There was a problem hiding this comment.
Yes, in PCP it was added as a quick fix when we could not use PHP-Scoper yet for some reason. woocommerce/woocommerce-paypal-payments#972
So, here we have to choose which trade-off we prefer:
- A cleaner approach without Mozart, but possible issues with some third-party plugins in the development environment.
- The current messier approach, solving these issues.
- We can also try to isolate the third-party plugins instead, if there is a simple and reliable way. https://inpsyde.slack.com/archives/C01DTSYJ743/p1774274144957039
One of the issues with that Mozart approach is that it may be confusing which version of the package is actually used and it will not update automatically. So, if we are going to use it, maybe we should at least try to somehow automate it.
3 participants
Description
Migrate the plugin architecture from
dhii/module-interface+dhii/containerstoinpsyde/modularity ^1.7, and scope conflict-prone packages with Mozart.Container bootstrap — Replaced the manual container chain (
ProxyContainer → CachingContainer → DelegatingContainer → CompositeCachingServiceProvider) withPackage::new(PluginProperties).All 18 modules now implement
ServiceModule,ExtendingModule, and/orExecutableModulewithModuleClassNameIdTrait, returningservices()/extensions()arrays directly instead of wrapping inServiceProvider. Boot logic is inlined intorun().Library classes — Migrated
QueueLibrary,StateMachineLibrary,WcEventsLibrary, andZettlePhpSdkLibraryfrom Dhii container chains toPackage::new(LibraryProperties::new(...)).Interface replacements — Replaced
dhii/collections-interfacewith project-levelWritableContainerInterface/ClearableContainerInterfaceinsrc/Container/. Replaceddhii/validatorwith project-levelValidatorInterfaceinsrc/Validation/. Replaceddhii/event-dispatcher-interfacewith PSR-14 (psr/event-dispatcher).Cleanup — Removed all Dhii packages from root and 18 module
composer.jsonfiles. Updated test infrastructure and fixed extension argument order in test files.Mozart namespace scoping — Scoped
psr/containerandinpsyde/modularityintolib/packages/under theSyde\Vendor\Zettle\prefix using Mozart, matching the pattern from PayPal Payments.Key behavioral change: In Inpsyde Modularity, extension callbacks receive
($previous, $container)instead of Dhii's($container, $previous). Extensions may receivenullas$previousif no factory is registered for the service key.Steps to Test
ddev composer installto verify dependency resolution with no Dhii packagesddev exec vendor/bin/phpunit— expect 31 passed, 3 incomplete, 2 skipped (7 auth errors require API credentials, 1 pre-existing state machine failure)wc-smooth-generatoror other plugins that bundlepsr/container1.x