-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Sideload root only #353
Sideload root only #353
Conversation
I agree with @ahawkins that we should consider this as a more sane default than to load and serialize everything. The test case that was added makes the reasoning pretty clear. Here is a simple recap using this gist (basically just user has many posts): # With this PR and using the include_root_only: true flag the output is:
{
post: { id: 1, title: 'Post 1', author_id: 1 },
authors: [{ id: 1, name: 'Adam Hawkins', post_ids: [1,2] }]
}
# Without this PR the output is:
{
:post=>{:id=>1, :title=>"Post 1", :author_id=>1},
:authors=>[{:id=>1, :name=>"Robert Jackson", :post_ids=>[1, 2]}],
:posts=>[{:id=>1, :title=>"Post 1", :author_id=>1}, {:id=>2, :title=>"Post 2", :author_id=>1}]
} |
@steveklabnik ping |
I'm awaiting @spastorino 's stuff to land before merging anything. |
I'd even suggest this should be the default behavior. |
👍 for default behavior. Thoughts on |
The way I've handled this is to define 2 serializers for each resource. I end up with one serializer used for For example, I have a def index
render json: monkeys
end
def show
render json: monkey, serializer: DeepMonkeySerializer
end |
We are going to implement contexts for Serializers that also will help solving this issue so I'm closing this for now. |
Give an option to include associations from the initializer serializer only. This is good when you need to serializer many objects that are all related, but don't want to include the entire graph.
I wonder if this should be on by default. All serializers sideloading everything is path for serious performance problems.