Do not lose the goldiloader with additional includes
Closed this issue · 2 comments
By calling something like:
Company.all.each do |company|
company.employees.includes(:person).to_a
end
We lose the goldiloader here because of the includes
.
Although, the includes itself shouldn't cause any issues if we somehow simple merge it with the detected includes
.
Another approach is to get rid of every single includes
from the app if we rely entirely on goldiloader, but that's heavy compared to only adding the gem 🤷
Thanks for filing this issue @sobrinho. Unfortunately calling includes
(or any other methods from the Active Record query interface method like where
) on an association creates a new query scope that is independent from the association so none of the existing Goldiloader code for detecting when to perform automatic eager loading on that association will work. I don't think we want to change this since it would break scenarios where you want to force the loading to happen at a particular point in the code (e.g. before deleting records from the underlying database) or change the query strategy (e.g. using joins).
I would recommend explicitly removing includes
from your application where it's safe to do so (a tool like synvert could help automate this). Alternatively, you could monkey patch ActiveRecord::Associations::Association#includes
to be a no-op but that could have unintended consequences.
Sounds good to me, thanks!