Feature/prepend configs

[magarzon]

Added prepend methods to BundlePlugin interface and ExtensionWithPlugins (having this class implement PrependExtensionInterface), so plugins have an opportunity to prepend configuration (method is called in container compilation time)

Use case:

• A bundle SBPBundle that use another third party bundle, with a default configuration.
• The SBPBundle it's used in many apps, that should repeat that third party bundle configuration.
• Set default third party bundle configuration in prepend method in a CorePlugin, so no need to repeat that default configuration in every app that use SBPBundle

[matthiasnoback]

I'm totally convinced of the need for the prepend() method, thanks for proposing it. Could you think of a way to demonstrate the new feature in a test?

As a note to myself, this would require a new major release, since changes are BC-breaks (when upgrading, people will have to implement the prepend() method as well).

[Spomky]

This is not necessary for BundlePlugin interface to implement PrependExtensionInterface.
In the prepend method of ExtensionWithPlugin, you just have to check if the plugin implements PrependExtensionInterface:

//src/ExtensionWithPlugins.php

public function prepend(ContainerBuilder $container)
{
    foreach ($this->registeredPlugins as $plugin) {
        if ($plugin instanceof PrependExtensionInterface) {
            $plugin->prepend($container);
        }
    }
}

With this way, only one file is modified and plugins do not need to have a void prepend method.
Moreover, you will not have any BC-break as this method is not required.

[magarzon]

@Spomky you're right, but boot() or build() aren't needed too, and there are in the BundlePlugin interface, so I added prepend() method in the same way: added to the interface and implemented (void) in SimpleBundlePlugin

[Spomky]

Yes in some cases these methods aren't needed but executed.

---

Off topic:

What is the best way (performance, DX)?

1. create empty methods and execute them
2. check if a class implements an interface and execute the desired methods

With solution 1., the BundlePlugin interface has to be splitted into small interfaces. Plugins will just have to implement required methods.

@matthiasnoback what do you think if new interfaces are created to avoid empty methods?

...
interface Loadable
{
    public function load(array $pluginConfiguration, ContainerBuilder $container);
}

...
interface Configurable
{
    public function addConfiguration(ArrayNodeDefinition $pluginNode);
}

...
interface Buildable
{
    public function build(ContainerBuilder $container);
}

...
interface Bootable
{
    public function boot(ContainerInterface $container);
}

[matthiasnoback]

Thanks for brining this up.
It's a very simple library anyway, so I don't feel like making many structural changes. It's a good idea to have prepend extension stuff in this library, so it also makes sense to split the extension in the way Symfony splits it as well: a separate interface for plugins which have a prepend() method. This still requires the ExtensionWithPlugins class to implement PrependExtensionInterface, but it allows us to have no BC-breaking change.

[xabbuh]

What is the best way (performance, DX)?

1. create empty methods and execute them
2. check if a class implements an interface and execute the desired methods

I would not care too much about performance here as this code path is only executed when/before the container is compiled and from the developer's point of view I'd say it's easier to implement an extra interface when you need that functionality than to let every plugin developer bother with another method they don't really need.

[matthiasnoback]

Agreed, however: having all these interfaces is not necessarily a good thing. I don't like all the if (... instanceof ...) that would be required in many places. Role interfaces are a good thing, but on a per-method basis it leads to less cohesion (because of the splintered hierarchies).

[Spomky]

Should be closed as already implemented by #12

[matthiasnoback]

Okay, I had forgotten about this entirely. I'll trust you on your word @Spomky :)
Please reopen or add a comment if I misunderstood this.