search

rails-api / active_model_serializers

ActiveModel::Serializer implementation and Rails hooks
MIT License
5.32k stars 1.39k forks source link

[0.9 stable] Revert "Allow serializer_for to accept String instead of just class objects" #2468

Closed casperisfine closed 2 months ago

casperisfine commented 2 months ago

Ref: https://github.com/rails-api/active_model_serializers/pull/2446/commits/460d4c564af5e45661a30ed1a40e797e4bdda6f4

I first thought about making it configurable, but I just can't find a justification for this feature in a very stable gem with such backward compatibility impact.

cc @bf4

@Physium could you tell me more about your use case? Because this change broke our app quite hard, in a cause we had:

render json: "Error"

Which caused AMS to look for ErrorSerializer, which really we don't want.

Can't you just do: 

render json: Custom.new

instead of:

render json: "Custom"

?

Physium commented 2 months ago

@byroot apologies for the inconvenience. I dont recall the exact issue as to why I did this but its due to a ton of legacy code while I was trying to bring an 8 year old application back to life. I'm good with reverting this and will open another issue/pr again if your recommendation doesnt work out well.

@bf4 apologies, i think i took that pr to address issues within my application a little too extreme! looking back at it again it should have been a different PR.

Should we merge this?

casperisfine commented 2 months ago

apologies for the inconvenience.

No worries.