Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

Pitfall: Auth::user()->with('company')->first() #2642

Open
kw-pr opened this issue Jun 16, 2021 · 4 comments
Open

Pitfall: Auth::user()->with('company')->first() #2642

kw-pr opened this issue Jun 16, 2021 · 4 comments

Comments

@kw-pr
Copy link

kw-pr commented Jun 16, 2021

Today I tried Model::preventLazyLoading(! app()->isProduction()); and walked right into this pitfall.

I changed

$user = Auth::user();

to

$user = Auth::user()->with('company')->first();

While the first line returns the logged in user.
The second line returns the first user in the database.

After I figured out my mistake I felt stupid about it, but I talked to some other developers and they all had the same WTF moment.

Well, if you like me did not realize the problem: with is a static function that is called on the already loaded user object. Like User::with('company') this creates a new query that has nothing to do with the user returned from Auth::user().

This actually a PHP "problem" as you always can call statics functions from the object.

I was wondering how this pitfall could be prevented and found a magic solution.
As this is the real world I am not very happy with magic hacks to fix a problem.

What do you think about this?
I am the only idiot and this does not need to be fixed or do you agree?
Maybe even have a good solution?

@kw-pr kw-pr changed the title Pitfall: Auth::user()->with('company)->first() Pitfall: Auth::user()->with('company')->first() Jun 16, 2021
@rognales
Copy link

@kw-pr
Copy link
Author

kw-pr commented Jun 17, 2021

Yes, I know. This works:

$user = Auth::user()->load('company');

Still $user->with('company') returns a unexpected result.

@michaeldyrynda
Copy link

Auth::user() returns the User model for the currently authenticated user, not a query builder instance.

Running Auth::user()->with('company') is akin to running something like User::firstWhere('id', $id)->with('company').

Chaining first() on a model instance is essentially the same as calling User::first().

As above, if you already have a model instance, the appropriate course of action is to call ->load('company') on it, as eager loading works only on the Eloquent query builder. Note that calling load() won’t be prevented by preventLazyLoading() as - although lazy - it is still considered eager loading.

The lazy loading being prevented by that functionality would be calling Auth::user()->company, which would otherwise query the database if the relationship was not already loaded.

@kw-pr
Copy link
Author

kw-pr commented Jun 17, 2021

Auth::user()->with('company') does nothing with the $id. It is a static method and can not access the object at all. That is kind of my point.

I am not looking for a method to load my relationship. load() works fine.

I am complaining that the code does not what it looks looks like from a clean code perspective.

$user->with('company') should load the relations (as it looks like) or in this case maybe better throw an exception. Calling the query builder on the object is wrong. This only works because of a PHP forgives everything quirk. It is a static method. It has nothing to do with the object.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants