Add Service Provider Extender

[SychO9]

Fixes #2421

Confirmed

• Backend changes: tests are green (run composer test).

[clarkwinkelmann]

Looking good code-wise.

Not sure we need a dedicated exception for the tests that doesn't contain any logic though? We could just throw an \Exception directly?

[askvortsov1]

Looks good to me, thank you!

[franzliedke]

Sorry I'm a bit late to this game, but I cannot help it...

I believe this extender is not a good idea, for a few reasons:

1. This does not seem use-case driven. Sure, somebody may have said "I wish I could register/replace/manipulate service providers". But that's not a purpose in itself. You always use service providers to do something - that something is interesting. It doesn't necessarily require messing with service providers, and if it does, it might warrant an explicit extender instead.
2. An even bigger problem, IMHO: it significantly increases what can be considered public API - with this extender, basically all of the service providers used by Flarum itself are now part of the public API - their existence, whatever they register and so on. But they are glue code, they can be (re-)structured at will, and one of the main point of the extender API was to prevent such internal restructurings from breaking extensions.

If extension devs want to mess with service providers, they can do so by writing their own extenders. But doing so clearly signals "You're doing this at your own risk." That's good, if you ask me. 🙂

[askvortsov1]

No worries, your feedback is always appreciated and super valued!

I'm a bit torn on this one. On one hand, I definitely see where you're coming from, especially with the expanded public API point. But on the other hand, the purpose of an extender is to be used (and reused) by extensions to modify core. IMO, an extender is something that core (or any extension) provides to expose the public API of it's backend to other extensions. I also believe that when users build hacks or modifications around a product to get it to accomplish something, that's a pretty clear indication that the feature / capability in question is in need. Putting these things together:

• I am concerned that a large number of extensions need to use custom extenders for modifications that aren't really "extender territory"
• I do not think that custom extenders are the right way to provide this advanced access, as that further complicates extensions which genuinely publish extenders to, well, extend the extension (like we'll see when search drivers are implemented). These are 2 very different use cases, and should be provided through separate channels IMO.

To that end, I think that this PR is a necessary evil, but I would be in favor of adding a bunch of warnings to it disclaiming that the only guarunteed public API is the official core extenders, and that this is provided as a courtesy to extension developers for advanced usages done at one's own risk only. I would also like to monitor community usage of this extender (perhaps via query.flarum.dev?) to see if any of those usages should receive official public API extender support.

Thoughts @flarum/core @SychO9? Sorry for the excessive tag, but this is an important side of this issue that we should come to a consensus on before b15 is released.

[SychO9]

@askvortsov1 Why would an extension developer need to create a custom extender (class implementing the ExtenderIntefrace) just to have access to the container ? They can just use a closure to directly manipulate the container, or register a service provider from there.

As far as I know there is no difference in the order of execution between using a custom extender and a closure where you either register a service provider or directly do things and a registered service provider. A service provider has the added ability of running code after the app is booted through the boot method.

return [
    function (Application $app, Container $container, ...$otherDependencies) {
        $container->bind(...);

        $app->register(CustomServiceProvider::class);
    },
];

[askvortsov1]

The closure system is deprecated and removed with stable

[SychO9]

I see, I wasn't aware of that. Then I would have to agree with this:

To that end, I think that this PR is a necessary evil, but I would be in favor of adding a bunch of warnings to it disclaiming that the only guarunteed public API is the official core extenders, and that this is provided as a courtesy to extension developers for advanced usages done at one's own risk only.

We would need to make sure to properly document this.

It would be a mess if we started creating custom extenders, when they are not actually for extending the extension in question. One that comes to mind is the pusher extension.

Also, I'm not entirely sure that leaving extension developers to use custom extenders does clearly signal to them that they're doing it at their own risk.

[askvortsov1]

Also, I'm not entirely sure that leaving extension developers to use custom extenders does clearly signal to them that they're doing it at their own risk.

Either way, documentation that this has NO BC guaruntee is necessary, so we might as well make it easier (and less confusing with no need for custom, single-use extenders).

[clarkwinkelmann]

Right now I have two main reasons I use providers/"local extenders":

• Grouping event listeners
• Registering libraries in the container

The first use case gradually fades away as more and more features are exposed through extenders instead of events.

The second use case however won't disappear. Service providers are an important part of the Laravel container. I like to define my own singletons, class aliases, and sometimes I intentionally want to override classes from Flarum or other extensions at the container level.

If callback-based "extenders" go away and "local extenders" are discouraged, there would be no official way to register service providers anymore. So we need this extender.

[askvortsov1]

In that case, @franzliedke unless you have further objections I'll proceed with a PR that adds a warning that the service provider level is NOT officially considered public API, and is subject to unannounced BC breaks.

[clarkwinkelmann]

We need a separate issue to discuss what we consider public API in the container. We've discussed things like hasher are intended to be overridable by extensions, so we probably should have a list of the container bindings we consider public, and have extensions consider anything else as private.

Access to the settings interface, translator interface, validator interface, hasher, should probably be considered for BC if we change them after stable.