[8.x] Relax the lazy loading restrictions

[themsaid]

This PR will only throw the exception if the models being hydrated are more than 1. If it's only a single model then it means N in N+1 will always be 1. So no problem here.

[amcsi]

So to clarify, will this mean that...

$posts = Posts::limit(5)->get();
foreach ($posts as $post) {
	$comments = $post->comments;
}

...the above will trigger the lazy load protection, but...

$post = Posts::findOrFail(1);
$comments = $post->comments;

... will not?

Because then that's really cool.

[themsaid]

@amcsi correct

[juliomotol]

I may be a bit late but we've noticed something and would like to clarify if it is the intended behavior given the ff cases:

Post has 5 Comment

$post = Posts::findOrFail(1);
$comments = $post->comments; // is allowed
$comments = $post->comments->first()->likes; // is not allowed

Getting Comment's related Like is not allowed since during hydrate() we found >1 $items

Post has only 1 Comment

$post = Posts::findOrFail(1);
$comments = $post->comments; // is allowed
$comments = $post->comments->first()->likes; // is allowed

Getting Comment's related Like is allowed since during hydrate() we only found 1 $items hence not preventing to lazy load

[Matslom]

So, when I set preventsLazyLoading and forgot specify with on query. Then on some results, when there will be more than one item I will get an exception, and on some not?
That feature leaks of consistency, don't you think? Potential bugs in code. If it works that way only real option to preventsLazyLoading is to set it only on local env...

Also, why that count($items) is inside array_map? Does every iteration need counting?

[amcsi]

@Matslom I agree with you. It would be better if PreventsLazyLoading didn't check whether there was more than 1 item in the collection for determining laziness violation, but rather if it was a result that came from a query that did not use ->first() or ->last(). But maybe it works this way, because there's no way to differentiate them.

In any case the way it works now is better than nothing. We get at least some warning/reminders about performance, even if it's a bit random.

[dennisprudlo]

@Matslom

If it works that way only real option to preventsLazyLoading is to set it only on local env

You should use this feature only in local environments either way. This way you will be notified when there is some room for optimization.

I can't think of any added value when this is being used in a production environment except it is very critical to ones infrastructure that N+1 is prevented. As per the docs:

you may wish to only disable lazy loading in non-production environments so that your production environment will continue to function normally even if a lazy loaded relationship is accidentally present in production code

Even with consistent behavior you wouldn't want to present your user an error page just because you forgot some lazy-loading case.

[Matslom]

You should use this feature only in local environments either way

vs documentation

you may wish to only disable lazy loading in non-production environments so that your production environment will continue to function normally even if a lazy loaded relationship is accidentally present in production code

may wish !== should, because it may create problems over time

I'm creating app from scratch. First thing that I did was disablingLazyLoading. By reading the docs I didn't see case, why should I do this only on local env.
IMHO Docs should describe that "small" inconsistency created in this PR.

Even with consistent behavior you wouldn't want to present your user an error page just because you forgot some lazy-loading case.

If it would be consistent, then I don't see case when I ship some lazy-loading cases that can break over time.

[dennisprudlo]

Of course you can enable it on production. Just saying I think there are very few cases where I would actually want to, because you throw a server error where there's actually no error. It's just inefficient code in most cases.

The app can still fallback to lazy loading without every breaking, like we log warnings instead of crashing the app intentionally.