elephpant.me
elephpant.me copied to clipboard
perf: only execute database query when needed
This ensures the CountriesQuery is only executed when the user actually lands on/loads any of the composed views. Previously, this would always execute the query (on every request) because it was done outside of the closure.
we have pros and cons with both approaches as I can see.
-
keeping the query outside the closure requires a single call on each request, and all views using it will have it already resolved, correct? so the worst case we will have a single query per request
-
moving it to inside the closure will make the query to be made only on request that really needs to use countries (good), but if we have on the same request more than one view (or partial) using the countries I guess it will query it more than one time, correct? so I think on a
@include('_user')call we might have multiples queries for the # of partials required we have
let me know what are your thoughts here please 💪 thank you so much
Ah good points @jgrossi! I'm actualy not sure if Laravel already caches the result of a view composer, I'll have a look at the internals later and report back 😉
If Laravel indeed doesn't cache those results, what do you think of using spatie/once for this? Or would you rather we not pull in another dependency just for memoization? We could solve this pretty easily ourselves by "caching" the result of the query on a static class property.
both ideas are good. spatie/once also uses static properties to cache the result in the same request. for me no problem of adding the package to composer. go ahead
Hey guys, what's missing to merge this PR?