Serailize plain arrays

[mitchlloyd]

This PR is in response to #772

I wanted to learn about ActiveModelSerializers so I thought I'd try to tackle a bug. Unfortunately the one I picked lead me to some pretty big design questions.

Specifically:

Is there a way to avoid invoking all the guts of AMS on a "plain" array?

class PostsController < ApplicationController
  def show
    # Would be nice to avoid AMS altogether
    render json: ['one', 'two', 'three']
  end
end

and what does it even mean to have primitive values in an array with serializable objects?

class PostsController < ApplicationController
  def show
    post = Post.find(params[:id])
    render json: ['look at this post', post, 3]
  end
end

I'm sending this PR because it at least provides a failing test for the issue. If you have some helpful insights or If you think this PR might be usable let me know and I can work on any needed adjustments. Otherwise, feel no remorse about closing :)

[kurko]

Take a look at https://github.com/rails-api/active_model_serializers/blob/master/lib/action_controller/serialization.rb#L22. You can redefine user_adapter? in your controller and disable it temporarily.

[solher]

Works great for me ! Thanks a lot @mitchlloyd. But your fork isn't going to be merged ?

[kurko]

@mitchlloyd does my answer help you?

[mitchlloyd]

@kurko I think you may have misinterpreted my intent. I was trying to get the ball rolling on fixing an issue that you identified as a bug. I'm not scratching my own itch.

Are you suggesting that issue #772 could be solved with better documentation?

I was mainly looking for feedback on this PR.

[hderms]

I was going to fix the same issue #772 as @mitchlloyd but realized he already had done so. Is there any obstacle to getting this merged in?

[kurko]

@mitchlloyd so, if you have MyUser model and no MyUserSerializer, it will set DefaultSerializer? I'm not sure about that. If you're serializing a model that has no serializer, I believe it should fail.

The original bug (#722) was regarding arrays, instead.

We need more opinions on this.

[mitchlloyd]

@kurko I can see how throwing an exception when an object doesn't have a serializer could be a better experience than falling back to the default serialization behavior of Rails in some cases. I agree it would be nice to get more opinions on this though. If that's the conclusion then I'd be happy to change the PR to implement that behavior.

However, in this particular case (from #772):

render json: [ 'No session found for the current user' ]

Should the user see an error? Error: no StringSerializer was defined. Probably not. Here the OP just expected the default JSON serialization behavior from Rails which seems pretty reasonable to me.

Another possibility might be to do a check upfront for primitive values like String and Hash and not attempt to lookup a serializer for those values. That seems like it could be a little fragile, but let me know what you think.

[kurko]

@mitchlloyd yes, I think we should whitelist those primitive types and have a baked-in serializer for them. As for the rest, my opinion is that we should just raise.

[mitchlloyd]

This new PrimitiveSerializer handles integers, strings, hashes, and symbols. I kept this check after the primary use case of class-name-lookup for objects with user-defined serializers. I didn't want to add anything that would hurt the performance of the primary use case of the gem.

We still have the behavior where users will get an sub optimal error (NoMethodError (undefined method 'new' for nil:NilClass) if they don't have a serializer defined for a given class. However, that's a different issue that probably warrants a new PR.

[bf4]

Propose to close as fixed by #962 (though I'm not 100% sure that is true)