laravel-streams / streams-core

Streams is an open-source web application engine for Laravel.
https://streams.dev
Other
169 stars 99 forks source link

StreamServiceProvider::registerAddons #740

Closed RobinRadic closed 1 year ago

RobinRadic commented 2 years ago

The composer vendor dir path might be usefull in other places, like third party addons. I recommend

        $composer = json_decode(file_get_contents(base_path('composer.json')), true);
        $lock = json_decode(file_get_contents(base_path('composer.lock')), true);

        if ($directory = Arr::get($composer, 'config.vendor-dir')) {
            $directory = base_path($directory);
        }

        if (!$directory) {
            $directory = base_path('vendor');
        }

the directory value coming from this to be bound in container. So it is re-usable. No need to check this twice.

I also recommend adding the JSON_THROW_ON_ERROR flag:

json_decode(file_get_contents(base_path('composer.json')), true,512,JSON_THROW_ON_ERROR);

This will make developers not only aware that something is wrong, but where the error is located

RobinRadic commented 2 years ago

Also, why not use the previously registered 'singleton' composer.json that is registered in registerComposerJson;

IMO. when you are already calling this same code in registerAddons, you might want to make registerComposerJSon into a $this->app->instance('composer.json', json_decode(file_get_contents(base_path('composer.json')), true,512,JSON_THROW_ON_ERROR));

Because it is certain that its going to be accessed later in the lifecycle

RobinRadic commented 2 years ago

Also, anything that is called from boot, in term of naming, i think it would be better to:

bootAddons instead of registerAddons as it is called from the boot lifecycle function.

RyanThompson commented 1 year ago

This has since been cleaned up and implemented similarly to as mentioned here.

Pixney-William commented 1 year ago

This has since been cleaned up and implemented similarly to as mentioned here.

Gr8