[0.9 stable] Revert "Allow serializer_for to accept String instead of just class objects" #2468
Conversation
…bjects" Ref: rails-api@460d4c5 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.
casperisfine
force-pushed
the
0-9-stable-revert-string-serializer
branch
from
0325a86 to
aac72cf
Compare
| @@ -49,7 +49,7 @@ def test_serializer_for_array_returns_appropriate_type | |||
| def object.serializer_class; CustomSerializer; end | |||
|
|
|||
| assert_equal CustomSerializer, Serializer.serializer_for(object) | |||
| assert_equal CustomSerializer, Serializer.serializer_for('Custom') | |||
| assert_nil Serializer.serializer_for('Custom') | |||
There was a problem hiding this comment.
I agree with reverting this behavior change
I either missed or misunderstood it when originally reviewing 460d4c5 in https://github.com/rails-api/active_model_serializers/pull/2446/files
|
@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? |
No worries. |
Ref: 460d4c5
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:
Which caused AMS to look for
ErrorSerializer, which really we don't want.Can't you just do:
instead of:
?