Extension dependencies feedback

looky

While reading through @FilipProcházka 's Kdyby/PresentersLocator extension, specifically this part, I got an idea of making extensions able to define dependencies on other extensions. I tried creating something here, and I would like some feedback, if it's even worth a PR. What do you guys think?

Last edited by looky (2014-11-19 14:18)

Filip Procházka

I like that you're tryting to solve it. But I think this might be overkill. Simply assinging “weight” or “priority” to the extensions and then sorting them should be enough.

looky

I considered that. What priority would you assign to default extensions? How would you allow inserting your extension between two default extensions?

I also considered simple “require” (which is basically “after” dependency), but that wouldn't solve PresentersLocator's issue. Unless you could somehow redefine dependency for default extensions, which I guess could be done..

xificurk

How about moving this logic to ExtensionsExtension. Configurator would then instantiate only ExtensionsExtension, pass it the list of default extensions and let it handle the rest… The list of default extensions (including their order) could be then altered directly from config.neon.

hm… it seems that won't work, because Nette\DI\Config\Helpers::merge() keeps the order of items in associative arrays from $right argument and not $left.

Last edited by xificurk (2014-11-17 21:17)

xificurk

I think https://github.com/…e/di/pull/30 and https://github.com/…e/di/pull/31 solve most of the use cases.

looky

@xificurk The first pull is awesome, but I don't really like the solution from the second one. I agree with David's reasoning. Maybe I could keep the logic I have, but move it from config to the code itself, something like getBeforeDependencies() and getAfterDependencies() methods that would return lists of extension names?

Also, should it not be extension classes' names instead of just names?

xificurk

@looky It should be definitely extension classes, not names. And the checks should work even if the extension class is only a subclass of the specified class name, or implements an interface of the specified name.

And yes, the coupling logic should be moved from config to Compiler itself. I'm not sure if we should add the methods you mentioned directly to CompilerExtension or introduce a new interface that an extension might optionally implement to indicate that it needs to be processed before/after a specific extensions.

By the way, your original proposal suffers from the same problem as my PR 31 – it screws up the iteration logic in Compiler::processExtensions().