rails-api / active_model_serializers

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

ActiveModel::Serializer.serializer_for unable to search for serializer with base model after Ruby upgrade from 2.5 to 2.7 #2443

Closed Physium closed 1 year ago

Physium commented 1 year ago

Current I have a model that references a base model and a serializer that ties to the base model. Before upgrading from ruby 2.5 to 2.7, ActiveModel::Serializer.serializer_for is able to return the base serializer with no issues. After upgrading, it doesnt seems to detect the serializer base on the base model. I cant seem to find anything related in the changelogs as well.

Currently I'm on rails 5.2-stable with active_model_serializers 0.9.4.

Example:

Class Dog > Animal

Class AnimalSerializer < BaseSerializers

Ruby 2.5
ActiveModel::Serializer.serializer_for(Dog)
=> AnimalSerializer

Ruby 2.7
ActiveModel::Serializer.serializer_for(Dog)
=> nil
bf4 commented 1 year ago

If you can help write a failing test and are will to try and help by contributing a PR, I can facilitate.

Physium commented 1 year ago

If you can help write a failing test and are will to try and help by contributing a PR, I can facilitate.

I'm open to that. But before I dive deep, can I do a quick clarification if v0.9.x supports ruby 2.7? Also I understand that theres a difference between v0.9 and v0.10 and it seems like its recommended to upgrade from v0.8 to v0.10 but not v0.9. Are they both serving a different use-case?

bf4 commented 1 year ago

It should support it, but it's been years since it was actively developed. 0.9 was a rewrite of 0.8 and 0.10 was supposed to start out based on 0.8 but didn't have a good upgrade path. 0.9 is probably fine if it works for you and you're already using it. I got involved during the 0.10 rewrite. I'm just here now to facilitate.

On Mon, Mar 27, 2023, 9:16 PM Wei Jun @.***> wrote:

If you can help write a failing test and are will to try and help by contributing a PR, I can facilitate.

I'm open to that. But before I dive deep, can I do a quick clarification if v0.9.x supports ruby 2.7? Also I understand that theres a difference between v0.9 and v0.10 and it seems like its recommended to upgrade from v0.8 to v0.10 but not v0.9. Are they both serving a different use-case?

— Reply to this email directly, view it on GitHub https://github.com/rails-api/active_model_serializers/issues/2443#issuecomment-1486107631, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABC4QT5GY73MCR6SN5USRDW6JCWDANCNFSM6AAAAAAWI7CNYM . You are receiving this because you commented.Message ID: @.***>

Physium commented 1 year ago

Apologies for the late response.

I did an initial round of digging and found out that main root cause was due to the usage of _const_get in v0.9.x here

Going from ruby 2.5 to 2.6 onwards will result in the example mentioned below to not work.

Class Dog::Animal > Animal; end

Class AnimalSerializer < BaseSerializers; end

Ruby 2.5
# const_get is able to ignore Dog namespace and obtain the constant AnimalSerializer
ActiveModel::Serializer.serializer_for(Dog::Animal)
=> AnimalSerializer

Ruby 2.6 onwards
# const_get is strictly only looks for Dog::AnimalSerializer which does not exist
ActiveModel::Serializer.serializer_for(Dog::Animal)
=> nil

I cant seem to find any changelog in ruby that results in this behaviour but it seems that const_get became a lot stricter with how they match classes with namespaces as ruby progress in versions.

I notice quite a change in the implementation of serializer_for in v0.10.0 which uses safe_constantize and traverse down the super class if it does not matches. Not sure how practical if it if the implementation is copied over to v0.9-stable.

Would love to hear your thoughts.

Physium commented 1 year ago

@bf4 any ideas on how to move forward?

bf4 commented 1 year ago

I'll accept a pr. Does that work?