Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't auto-preload AGAIN during nested renders #16

Merged
merged 5 commits into from
May 30, 2024

Conversation

jhollinger
Copy link
Contributor

@jhollinger jhollinger commented May 21, 2024

Fixing an issue with auto and nested renders. When auto was on, nested associations were RE-preloading their subtrees. In many cases this was only a minor issue. But in certain situations (lots of manual preloads + field blocks that were really associations), it was a huge net loss of performance.

This is fixed by stricter matching in the extension. Preloading now only occurs if the rendered object's class is exactly ActiveRecord::Relation or ActiveRecord::AssociationRelation. Subclasses like ActiveRecord::Associations::CollectionProxy will no longer match, ensuring we don't preload on nested associations.

Since this is a significant issue, I've included a version bump so we can release asap. Also enabled appraisal in GH Actions.

@jhollinger jhollinger force-pushed the fix-nested-renders-with-auto branch from 7e3eca5 to 744b3c7 Compare May 21, 2024 17:13
Signed-off-by: Jordan Hollinger <[email protected]>
Signed-off-by: Jordan Hollinger <[email protected]>
@jhollinger jhollinger force-pushed the fix-nested-renders-with-auto branch from 0ebff74 to 8d17a10 Compare May 21, 2024 17:35
@jhollinger jhollinger marked this pull request as ready for review May 21, 2024 17:37
@jhollinger jhollinger requested a review from a team as a code owner May 21, 2024 17:37
@jhollinger jhollinger force-pushed the fix-nested-renders-with-auto branch from 391440e to 1cab8cc Compare May 28, 2024 23:37
@jhollinger
Copy link
Contributor Author

At @njbbaer's suggestion I attempted to use instance_of? instead of comparing strings. Sadly, this doesn't work as ActiveRecord is doing some magic:

q = Project.all
q.class.name
=> "ActiveRecord::Relation"

q.class
=> "Project::ActiveRecord_Relation"

@jhollinger jhollinger merged commit 7dbc5af into main May 30, 2024
7 checks passed
@jhollinger jhollinger deleted the fix-nested-renders-with-auto branch May 30, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants