More concise JSON serialization

[paulcsmith]

Just an experiment for now. Not sure how merge would work, but I think it is a nicer API.

EDIT: This is the old/original example, new proposals are in the comments!

class Users::Serializer < Lucky::Serializer
  needs user : User

  # Make this a class method instead of a new object
  def self.collection(users)
    { users: users.map { |user| Serializer.new(user) } }
  end

  # render single instance
  def render
    json.merge @user.to_h(:name, :email, :age)
    json.add "custom", "my-custom-value"
  end
end

# In an action
json Users::Serializer.new(user)
json Users::Serializer.collection(UserQuery.new)

[jwoertink]

Just thinking about this. I actually love the API we already have setup. It's setup easy to understand and use. Since we have the for_collection method, that already handles doing the collection.

Now, what I think might be cool is adding needs which is shorter and more consistent with other APIs, then we don't need to define an initializer (which I misspell ALL the time). Next, adding a quick macro to call render with some columns from the model.

class UserSerializer < BaseSerializer
  needs user : User

  # or whatever it would be called
  serialize name, age, username, email
end

That would essentially do this:

class UserSerializer < BaseSerializer
  def initialize(@user : User)
  end

  def render
    {name: @user.name, age: @user.age, username: @user.username, email: @user.email}
  end
end

and of course, the escape hatch here would be that you can just define render on your own. (maybe render should also be abstract since it's actually required 🤔 )

What are your thoughts on this?

[paulcsmith]

@jwoertink I like the idea of adding needs. I much prefer it over initialize in most cases TBH 😆especially since you can inherit needs.

I'm not as big a fan of a serialize macro because:

• It is hard to figure out how to modify the serializer with custom values
• macros in general have weird error message 😂

I think we can maybe solve this with regular methods, or potentially with a macro and really good error handling messages.

What if we had something like this where it kind of crosses between your suggestion and the original method based approach?

class Users::Serializer < BaseSerializer
  needs include_details : Bool = false
  needs user : User

  def render
     set({name: user.name, foo: "bar"}) # This is basically what you have today
     set(:single_key, 123) # Set a single key
     # Works great for conditionals
     if include_details?
        set(all_the_deets: true)
     end
     
     # macro  for setting multiple attrs
     set_attrs :email, :age, :other_thing, from: user
     # expands to
     set({email: user.email, age: user.age, other_thing: user.other_thing})

    # Also allow setting it under another key
    set_attrs :email, :age, :other_thing, from: user, key: :user
  end
end

Thoughts? I like this because the simple case is still there, you can see how to use conditionals pretty easily, and how to customize stuff. And it adds the set_attrs macro to serialize multiple methods from an object.

And since it is all still in a regular method changing the key name for a particular method is easy:

set_attrs :email, :age, from: user
set :name, "#{user.first_name} #{user.last_name}"

[jwoertink]

Ok, I had to read that example a few times to really process what was going on. I agree with your points on error messages. That's always the most important for us when it comes to implementing and changing stuff. One thing I want to try and avoid is something that reminds me of the days of using Rabl.

When you have some alternate syntax structure, but you're expecting your output to look a specific way, knowing what that output looks like could become troublesome. For example:

collection @posts
attributes :id, :title, :subject
child(:user) { attributes :full_name }
node(:read) { |post| post.read_by?(@user) }

Just looking at that code doesn't give you any sense of what the output json will look like. It's cool that you have tons of flexibility, but if you're sending this to a 3rd party API, and it's failing because a key is out of place, you might not get any helpful error that tells you what you're doing wrong. So for me, it's too many method names, and too much syntax.

I think I can get behind this... Let me know if this would work ok.

# complex example
def render
  set_attrs(:email, :age, :other_thing, from: user)
  set(name: "#{user.first_name} #{user.last_name}")
  set(single_key: 123)

  if include_details?
    set(all_the_deets: true)
  end

  set(preferences: {notifications: false})

  set(posts: PostSerializer.for_collection(user.posts))
end

# simple example
def render
  # throws error "unknown thing `post` on `Post`...?
  set_attrs(:id, :title, :post, from: post)
end

[paulcsmith]

Love it!! I like the idea of it taking any NamedTuple args and nesting it just like you expect. Nothing fancy, and no special overload for single arg 👍

And agree on the top level macros like that. We're on the same page I think. Those are hard to read and hard to customize. I've never been a fan of that which is why I did the simple approach to start haha.

[matthewmcgarvey]

Since Lucky::Serializer is only:

abstract class Lucky::Serializer
  def to_json(io)
    render.to_json(io)
  end
end

Does it make sense to transition it into a module? With this talk about adding in needs it seems like the CLI could generate a BaseSerializer that is the composition of the Lucky::Serializer and the Lucky::Assignable modules.

[matthewmcgarvey]

P.S. shouldn't the serializer declare the abstract def render?

[paulcsmith]

Yes it should declare abstract def and yes I think you’re right that it’d be better as a module!

[jwoertink]

yeah, I think it should. I even mentioned the same thing above 😅 Without the render method, it can't generate anything, so that makes sense.

[matthewmcgarvey]

With the module suggestion, I was also thinking about the opportunity that opens to move fast and include the Lucky::Serializer module directly in the models. While that is not recommended, I'm wondering if the method name of set is too generic? (referencing #526 (comment))

[paulcsmith]

We've got a BaseSerializer now so we could totally include Lucky::Serializer in it https://github.com/luckyframework/lucky_cli/blob/master/src/web_app_skeleton/src/serializers/base_serializer.cr

I'd like to avoid adding it directly to models just because that tends to lead down bad paths where you it is hard to customize, , but maybe I can be convinced! :D

Re set it is pretty generic, but since serializers are their own class I think it is ok and shouldn't conflict with anything. But open to other suggestions!