Skip to content

Conversation

@HLeithner
Copy link

No description provided.

@HLeithner HLeithner changed the title Exmaple of using $container for $dispatcher injection Example of using $container for $dispatcher injection Apr 30, 2025
@Fedik
Copy link
Owner

Fedik commented Apr 30, 2025

This can work indeed.
However looks hacky, kind of. Not sure about this approach.

But, booting the extension in to your own container can be a nice feature.

@HLeithner
Copy link
Author

which part would you say is hacky? parts which are b/c things or providing the container for booting the component?

@Fedik
Copy link
Owner

Fedik commented Apr 30, 2025

This part, which is b/c thing:

$container = Factory::getContainer();
if ($dispatcher) {
$container = $container->createChild()->set(DispatcherInterface::class, $dispatcher);
}
$plugin = Factory::getApplication()->bootPlugin($plugin->name, $plugin->type, $container);

But can work.

@HLeithner
Copy link
Author

That's actually not a b/c thing ;-)
We don't expect a $dispatcher, but inject it if provided, we always specify the container for the boot process.

There is no other possibility to provide the dispatcher to plugin. Because setDispatcher be called (here) and $dispatcher in the CMSPlugin::constructor has been removed.

@Fedik
Copy link
Owner

Fedik commented Apr 30, 2025

I somehow ignored that you changed the boot method signature, sure it is b/c 😄
I was thinking about code that help fixing b/c 😄

@Fedik
Copy link
Owner

Fedik commented May 3, 2025

I have another idea.
If we could reorganize bootPotato() form Application to dedicated ExtensdionManager class, we could do:

// Regular
$app->getExtensionManager()
  ->bootPlugin($name, $type);

// With custom container
$app->getExtensionManager()
  ->setContainer($container->createChild()->set(DispatcherInterface::class, $dispatcher))
  ->bootPlugin($name, $type);

This can be fully backward compatible, and provide much more flexibility.

What do you think?

upd. hmhm, but in this way we will have cashing issue, also with your suggestion in this PR

@HLeithner
Copy link
Author

yes I already found out, or better allon pointed me to the static cache in the boot method, which is not really useful in my opinion because it prevent you booting the same component in 2 different settings...

maybe it can be extended on the container hash... not sure

anyway your suggestion looks a way better then mine if it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants