Adding provider to bootstrap file causes issues deploying to production.

[danmatthews]

Hey Aaron, love solo.

We've installed it in our app, but because it's a --dev composer dependency, but registers it's service provider in bootstrap/providers.php, it causes issues when going to production when composer dev dependencies aren't installed.

I've removed it manually from the providers array, which fixes deployment, but then obviously breaks Solo locally.

One solution was to move the Solo::useTheme('dark')->addCommands([ call from the published provider into the AppServiceProvider - but obviously that has the same issue once you remove the --dev from your composer installs.

I think the route to go down here might have to be configuring all this in the config file instead of a published provider. Will fire up a PR if i get time (i'm away the rest of this week, but have friday free), but thought i'd drop this here incase a) i'd missed something or b) it sparked any thoughts in your own mind about how to handle this.

[fefo-p]

Same here.
Maybe conditionally registering the SoloServiceProvider?

[beblife]

Ran into the same problem, not extending the provided SoloApplicationServiceProvider and checking if it's autoloaded or not works.

<?php

namespace App\Providers;

use AaronFrancis\Solo\Facades\Solo;
use AaronFrancis\Solo\Providers\SoloApplicationServiceProvider;
use Illuminate\Support\ServiceProvider;

class SoloServiceProvider extends ServiceProvider
{
    public function register(): void
    {
        if (class_exists(SoloApplicationServiceProvider::class, true)) {
            Solo::useTheme('light')->addCommands([
                //...
            ]);
        }
    }
}

[fefo-p]

Worked for me!
Though you lose the ability of extending Aaron's service provider... At the moment it registers nothing, but that may change in the future

[joshmanders]

I ran into this issue too and was just checking PR's and issues to see if anyone else has.

I was also thinking of submitting a PR that has Solo use Laravel's auto descovery to register itself and pull from config/solo.php for declaring of stuff, for example this is how I have it in mine right now:

<?php

use AaronFrancis\Solo\Commands\EnhancedTailCommand;

return [
    'theme' => 'dark',

    // Commands that auto start.
    'commands' => [
        EnhancedTailCommand::make('Logs', 'tail -f -n 100 ' . storage_path('logs/laravel.log')),
        'Vite' => 'npm run dev --silent',
    ],

    // Not auto-started
    'lazyCommands' => [
        'HTTP' => 'php artisan serve',
        'Queue' => 'php artisan queue:listen --tries=1',
        'Scheduler' => 'php artisan schedule:work',
        'Reverb' => 'php artisan reverb:start',
        'Pint' => 'vendor/bin/pint --ansi',
    ],

    // FQCNs of trusted classes that can add commands.
    'allowedCommands' => [
        //
    ],
];

That way in production the only artifact that still exists for Solo is config/solo.php.

What are your thoughts @aarondfrancis? I can PR this asap.

[danmatthews]

@joshmanders do it, this is what I started doing the other day, but i've been on holiday this week.

9b24b92

[joshmanders]

@danmatthews I gotchu bro. Enjoy your holiday!

[beblife]

Worked for me! Though you lose the ability of extending Aaron's service provider... At the moment it registers nothing, but that may change in the future

Yeah, it's a temporary workaround to get things working again when deploying to production.

The config approach is probably the best way forward! ✌️

[aarondfrancis]

Question for y'all: using the config approach, a package could register a potentially malicious command without your knowledge. Minor possibility, but still a possibility. Do we care?

For example, someone might have a transitive malicious dependency that does the following:

config()->set('solo.commands', ['gotcha' => 'rm -rf']);

That's basically my only reason for preferring the service provider approach. What do yall think?

I think the move is probably to make the SoloServiceProvider not extend my (empty) application one, and just add an if check in there.

Thanks for your patience!

[joshmanders]

That is indeed a possibility, but the probability is very low.

[danmatthews]

Worth mentioning that EnhancedTailCommand would be missing if the package wasn't installed either, so would that fudge with the config file, it would certainly (i think) mess with config caching, even if configs are lazy loaded usually.

So if we went the config route, we'd likely have to find a way to represent that in another way too.

[aarondfrancis]

Mmm good point. Ok let's just fix the service provider stub then. If you want to use a config file you totally can! I've got one cooking over here anyway. In due time!

For now, I've change the provider stub. Since y'all have already published yours, you'll have to update it manually. Sorry about that.

Here's the commit that fixes the stub: 56cebb1

[joshmanders]

Worth mentioning that EnhancedTailCommand would be missing if the package wasn't installed either, so would that fudge with the config file, it would certainly (i think) mess with config caching, even if configs are lazy loaded usually.

So if we went the config route, we'd likely have to find a way to represent that in another way too.

Thats a good point, but that makes me wonder Aaron, with your work on the interactivity side, would that essentially make the EnhancedTailCommand irrelevant because now we can just use pint directly, which if I'm not mistaken is the reason why this EnhancedTailCommand was created.

[aarondfrancis]

Yeah it's a good question. Solo should be much more compatible with Pint now, including properly killing / cleaning up after Pint. I'll probably leave the Enhanced one there because it doesn't require another package, but I'll give Pint another go locally just to ensure it works

[joshmanders]

And by pint I meant pail 😬

I was just about to do some tests myself on it.

[aarondfrancis]

oh haha, me too

[aarondfrancis]

@joshmanders for Pail you'll need 0.2.1, because it still wasn't quitting.

#27

[joshmanders]

Excellent, I'll pull in latest changes and do some testing