[3.0] POC Dependency injection proposal

[MissAllSunday]

As explained in the development contributors board, this is a POC of how we might gradually integrate DI on SMF, starting with moving globals to services and eventually move actions to containers as well.

This POC is very raw, the action changes relies too much on static calls and autowire which is not optimal so I would rather focus on moving global and static calls (SMF\Config::$someVar) to services that can be injected where needed.

Documentation

• DEPENDENCY_INJECTION_MIGRATION_PLAN.md: Complete detailed plan
• DI_MIGRATION_QUICK_REFERENCE.md: Quick lookup guide
• DI_MIGRATION_SUMMARY.md: summary

This PR is not intended to be merged but rather use as a guide. Used AI to correct grammar and better orthography/summary/phrasing.

[Sesquipedalian]

Use of Lang::$txt is already deprecated in favour of Lang::getTxt()

[tyrsson]

Let's just be sure not to complicated this further in respect to eventually moving to PSR 7. All PSR 7 implementations provide a Url component. We should work to move in that direction whenever possible. Possibly simply by making sure that we are matching signatures with existing interfaces/behavior when possible.

[MissAllSunday]

Yes, as the other comments suggest, this is the initial step, once we have a clear picture of all the global vars used and moved to a more manageable state, we can now determinate which ones of those could be more suitable as something else. Certainly the scripturl and other uris are nice candidates to move to a proper component class (even the league has a nice package already) but we cannot go there without first moving all global state to dependencies.

[tyrsson]

I would not enable reflection resolution as it can introduce hard to track down dependency failures. I have diagnosed enough of them for Laminas/Mezzio to understand the implications (probably in the 100's). Just require them to be added via a Provider. Plus, it's much more performant.

[MissAllSunday]

yes, this is mainly to keep not migrated yet routes to still work as intended and not break the app while doing the migration. I mentioned here or elsewhere that routes are indeed part of the plan but require further and more in deep refactoring (precisely to avoid doing this kind of things). I want to focus on removing global usage first, get a nice set of decoupled services and then we can start thinking about routes.

[tyrsson]

Lets please NOT continue this into the container implementation. The container should not require a singleton implementation. Matter of fact doing so is probably one of the biggest code smells I can think of. The container itself exists to prevent this very thing.

I would also argue that this entire wrapper class is not needed. I need to spend a few minutes with the newer version of League\Container but will come back to this.

[MissAllSunday]

Yes, the container should not require a singleton or any wrapper calls, perhaps the guide did not put emphasis enough into it but the singleton and wrapper are temp solutions to maintain a functional state while the migration is been done.

The way the migration was envisioned was to make small adjustments instead of a large bulky PR containing everything and making it difficult to QA and implement, this is why we implement the container the same way SMF is calling other needed bits (static calls and singleton) until we are done with the foundation, then we can work on actually doing the needed refactor, including a proper Dependencies service provider.

The ultimate goal is indeed, to directly use the container without wrappers and without singletons, moving all the static calls to proper instantiations done via the container, perhaps I should add this to the guide to further clarify the goal.

[tyrsson]

If you address container construction in the entry and tackle setting up IoC for the Forum instance first and its means to provide a mechanism for invoking the Actions then all of the other work arounds are not needed. Those changes are all additive and should not break anything.

I will try to make time to provide an example this weekend.

[tyrsson]

@MissAllSunday
Just curious if you are aware that League\Container\Definition\Definition uses reflection even if you are not using ReflectionContainer explicitly. Definition uses reflection to resolve both arguments and callable factories.

This alone would be a no go for me. Symfony or Laminas either one would be much more performant. I am 100% in favor of PSR-11 but a better implementation should be chosen.

For reference:
League\Container\Definition\Definition

    /**
     * @throws ReflectionException
     * @throws ContainerExceptionInterface
     * @throws NotFoundExceptionInterface
     */
    protected function resolveCallable(callable $concrete): mixed
    {
        $resolved = $this->resolveArguments($this->arguments);
        return call_user_func_array($concrete, $resolved);
    }

    /**
     * @throws NotFoundExceptionInterface
     * @throws ReflectionException
     * @throws ContainerExceptionInterface
     */
    protected function resolveClass(string $concrete): object
    {
        $resolved   = $this->resolveArguments($this->arguments);
        $reflection = new ReflectionClass($concrete);
        return $reflection->newInstanceArgs($resolved);
    }

    /**
     * @throws ReflectionException
     * @throws ContainerExceptionInterface
     * @throws NotFoundExceptionInterface
     */
    protected function invokeMethods(object $instance): object
    {
        foreach ($this->methods as $method) {
            $args = $this->resolveArguments($method['arguments']);
            $callable = [$instance, $method['method']];
            call_user_func_array($callable, $args);
        }

        return $instance;
    }