search

spatie / laravel-medialibrary

Associate files with Eloquent models
https://spatie.be/docs/laravel-medialibrary
MIT License
5.78k stars 1.08k forks source link

Media collections are lazy loaded with `preventLazyLoading()` enabled #3554

Closed sebastiaanluca closed 5 months ago

sebastiaanluca commented 8 months ago

This concerns https://github.com/spatie/laravel-medialibrary/pull/2784

I noticed in our old admin panel that we had a lot of lazy loaded media relations. We didn't get a warning about it (for 2 years 😄), even with Model::preventLazyLoading(); enabled globally. The cause for this is that doing loadMissing() on each object in a loop doesn't trigger the exception even though it's technically an n+1 issue.

The solution to this is to always eager load relations manually in the controller, not in models or traits. Removing this would be a breaking change, but are you open to it? I expect a lot of package users have tons of n+1 media issues but aren't aware, unless you manually inspect the debug bar queries for each page you use media in collections.

freekmurze commented 8 months ago

I would be open for a PR 👍

sebastiaanluca commented 8 months ago

We'll try to look into it next sprint. Maybe we can also make this configurable so it's not a breaking change.

freekmurze commented 5 months ago

This has been fixed in the latest version.