Framework: Bug ? Eloquent queries in boot method crashes during composer install on fresh install

[poppabear8883]

• Laravel Version: v5.6.29
• PHP Version: 7.2
• Database Driver & Version: MySQL 5.7.*

Description:

Fresh installation on composer install crashes if Eloquent query is inside of boot method in a registered service provider.

example:
AppServiceProvider (boot method)

// Share $page across all views
view()->share('pages', Page::take(5)->get());

This is due to composer install executes php artisan package:discover near the end and it crashes on the query due to migrations have not yet been ran.

Example error output

Illuminate\Database\QueryException  : SQLSTATE[42S02]: Base table or view not found: 1146 Table 'unit3d.pages' doesn't exist (SQL: select * from `pages` limit 5)

  at /var/www/html/vendor/laravel/framework/src/Illuminate/Database/Connection.php: 664
  660:         // If an exception occurs when attempting to run a query, we'll format the error
  661:         // message to include the bindings with SQL, which will make this exception a
  662:         // lot more helpful to the developer instead of just the database's errors.
  663:         catch (Exception $e) {
  664:             throw new QueryException(
  665:                 $query, $this->prepareBindings($bindings), $e
  666:             );
  667:         }
  668: 
  669:         return $result;

  Exception trace:

  1   Illuminate\Foundation\Application::Illuminate\Foundation\{closure}(Object(App\Providers\AppServiceProvider))
      [internal] : 0

  2   Doctrine\DBAL\Driver\PDOException::("SQLSTATE[42S02]: Base table or view not found: 1146 Table 'unit3d.pages' doesn't exist")
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php : 82

[poppabear8883]

A work around could be something like this:

if (Schema::hasTable('pages')) {
    // Share $page across all views
    view()->share('pages', Page::take(5)->get());
}

[X-Coder264]

@poppabear8883 You don't need that workaround. You can achieve the same thing with the following piece of code:

view()->composer('*', function (View $view) {
     $view->with('pages', Page::take(5)->get());
});

Since the code that triggered your error in the first place is now in a Closure it's gonna be evaluated only when there's gonna be a need for that which means that php artisan package:discover won't throw exceptions anymore.

[poppabear8883]

@X-Coder264 thanks for the suggestion and the more elegant approach to solve the issue ... However am I wrong to assume that this is a issue with the framework ? I would think that this should be handled out of the box OR at least clearly documented ?

[sisve]

You're issuing database queries against tables that does not yet exist. You've written code that does this no matter if you will ever need the data. Your database query will fire even if the service provider is executed to find which artisan commands are available.

This isn't a framework issue.

[poppabear8883]

@sisve is this not a normal use case? Is this not meant to be used in this way. Its clear to me that it makes sense that one would want to "share" Eloquent data across all views. That being said, and knowing that any developer setting up the project would need to run composer install (because we dont VC the vendor directory) becomes an issue. I would think this is a very typical workflow that should be a framework problem to deal with.

Correct me if im wrong with my assumption that was the purpose of a view composer/view share ... to share data across all views, and we can assume that data is dynamic vs static ...

Not to mention its not very well documented for dynamic data. And the documentation clearly says to use these shares and composers in the boot method.

[X-Coder264]

@poppabear8883 Service providers are clearly documented how they work. Since your service provider isn't deferred it means that the register and boot methods of it will be called on each request (doesn't matter if its a HTTP or a console request). So that means that your query will be executed every time and of course if the table doesn't exist yet it'll throw an exception. This isn't a framework issue, the problem is how you use the tools it provided to you. You should use the composer method which accepts a Closure (unlike the share method which doesn't and is meant for simpler stuff) to prevent problems such as those (and you'll make that query only when it's actually needed for the view, instead of on every request).

[poppabear8883]

@X-Coder264 I appreciate the insight on the "proper" implementation. Would it be to much to ask for at least a dynamic example be placed in the documentation. Surely i am not the only one that wants to share dynamic data across all views using the service provider ...

We can see here that its not clear on how one would use eloquent. Yes the service providers are documented, it also has a very short explanation on the boot method ... neither of which addressing this issue.

[X-Coder264]

@poppabear8883 I created https://github.com/laravel/docs/pull/4412 for the docs, hopefully it's gonna be merged.

[lagbox]

(in the style of Crocodile Dundee)
"That's not a hack."

class HackThis implements \Illuminate\Contracts\Support\Renderable
{
    protected $callback;
    protected $value = null;

    public function __construct(callable $callback)
    {
        $this->callback = $callback;
    }

    public function render()
    {
        if ($this->callback) {
            $this->value = call_user_func($this->callback);
            $this->callback = false;
        }

        return $this->value;
    }
}

Some Service Provider @ boot:

view()->share('key', new HackThis(function () {
    \Log::info('one is the loneliest number');
}));

"That's a hack."

[lagbox]

In all seriousness though, a view composer really isn't the answer to this either. In this case that composer would get ran for every view that is rendered. Returning a view that extends a layout means you have at least 2 views rendered. So that composer would execute at least twice. You would have to design the function to get around that. So its not the same functionality as a share.

My 'hack' comment was too show misusing a feature [Renderable], hence the hack part, but it has the benefit of only being resolved once.

I am playing with some ideas to add something like a shareLater method or something similar at the moment.

[lagbox]

Update:
I should have a PR for this coming this week.

[lagbox]

Put it up on laravel/ideas first to see if I am missing anything

laravel/ideas#1289