Extensions: Drop support for Compat extender

[franzliedke]

Since introducing extenders in beta.8, we've still supported returning closures with auto-injection from extension's bootstrapper files (extend.php, previously bootstrap.php).

This fallback, implemented through the Compat extender, which has been marked as deprecated as long as it exists, can be removed.

[lukbukkit]

Is it okay to work on this issue or do want to wait until the beta8 is released?

[franzliedke]

@lukbukkit You are more than welcome to send a PR for this. However, we won't merge this until we have built extender classes for all known extension use-cases.

[franzliedke]

While we are at this...

We have discussed whether we want to keep this extender under another name a couple times, but haven't reached a definite conclusion. IMO, we should not make this stuff too easy.

Reaching into the container and using all the objects therein should not be considered public API. So, new idea: Instead of allowing closures with type hinted parameters or a Compat extender, can we just force people with custom use cases to implement their own extenders? Internally, we would only deal with extenders (yay for simplicity) and externally, this would be a non-trivial, but reasonably easy barrier to entry, which is just what we want to prevent this from being overused while still allowing full flexibility.

@luceos @tobscure Thoughts?

[luceos]

How would "implementing their own extenders" work if you can't use anything to hook into the App.. Or I'm missing something. Simple extensions don't need much, but looking at some of the more complex ones, eg Bazaar or console, need to be able to hook into the full app.

[franzliedke]

@luceos They would have to create a class that implements the ExtenderInterface: that's just one method, which immediately gives them access to the container. That is basically just as powerful (strictly speaking, even more powerful) than having the container do auto-injection on the current closures.

[luceos]

Right, somehow I assumed that everything has to be done using the extend.php file. But then again we're using autoloading 😴

[tobyzerner]

That sounds good to me. I assume it would support anonymous classes? so you can do:

return [
    new Extend\Whatever,

    new class implements Extend\ExtenderInterface {
        public function extend(Container $container, Extension $extension = null)
        {
            // do your thang...
        }
    }
]

[luceos]

A few suggestions based on re-using extenders in extensions:

• the Serializing one from PHP Extender: Serializing / forum api attributes #1624
• for adding gambits
• for adding policies
• for adding model relationships
• a Listen one to listen to extension specific or rarely used events:

<?php

namespace Flagrow\Byobu\Extend;

use Flarum\Extend\ExtenderInterface;
use Flarum\Extension\Extension;
use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Events\Dispatcher;

class Listen implements ExtenderInterface
{
    protected $listeners = [];

    public function extend(Container $container, Extension $extension = null)
    {
        /** @var Dispatcher $events */
        $events = $container->make(Dispatcher::class);

        foreach ($this->listeners as $listener) {
            $events->listen($listener[0], $listener[1]);
        }
    }

    public function on(string $event, $callable)
    {
        $this->listeners[] = [$event, $callable];

        return $this;
    }
}

[franzliedke]

👍 for the list, except for the event listener one. That makes it feel like we "support" listening to any of the events that we use as implementation detail. I consider them 100% private, I want to have the freedom to change them without any SemVer constraints.

Since building your own extender is so easy, as Toby demonstrated, I think that's a legitimate approach.

(It means we need to make progress on covering all current use cases with official extenders, though. Ahem.)