Skip to content
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

Should serializers be a struct? #1930

Open
jwoertink opened this issue Nov 10, 2024 · 2 comments
Open

Should serializers be a struct? #1930

jwoertink opened this issue Nov 10, 2024 · 2 comments
Labels
BREAKING CHANGE This will cause a breaking change

Comments

@jwoertink
Copy link
Member

In my main app, we've made all of our serializers structs; however, since we're currently using GraphQL, they're not quite the same as the default Lucky serializer

abstract class Lucky::Serializer

We've had these as a struct for at least 2 years with no issues (that I'm aware of). My thought is that a serializer is basically read-only. There's not really a time I can think of where you would want to alter the structure and pass it around all over the place. This may make a big difference in larger APIs. Does anyone have any thoughts on for or against? This would obviously be a fairly large breaking change that couldn't be backwards compatible.

@jwoertink jwoertink added the BREAKING CHANGE This will cause a breaking change label Nov 10, 2024
@akadusei
Copy link
Contributor

I've always used structs for serializers too, but I think it should be a module. This lets the end developer decide what's best for their use case:

module Lucky::Serializer
  abstract def render

  def to_json(io)
    render.to_json(io)
  end
end

Used as a class:

abstract class BaseSerializer
  include Lucky::Serializer
end

Used as a struct:

abstract struct BaseSerializer
  include Lucky::Serializer
end

@jwoertink
Copy link
Member Author

That's genius! 😄 Definitely a better way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This will cause a breaking change
Projects
None yet
Development

No branches or pull requests

2 participants