RFC: Name-spacing of objects under (module) ActiveModelSerializers vs. (core class) ActiveModel::Serializer

[maurogeorge]

Hi guys,

Today most of the public API is under ActiveModel::Serializer like:

• ActiveModel::Serializer.config
• ActiveModel::Serializer, which is the base class of our serializer

I think the only 2 public APIs that are under the ActiveModelSerializers is ActiveModelSerializers::Model and ActiveModelSerializers.logger, I think this 2 APIs need to live under ActiveModel::Serializer to keep consistency to the user. I created 2 PRs that is related with this #1291 #1296

In this PR #1138 was introduced a ActiveModel::Adapter::Base. I think we can do something like that, move from a class to a module and create a Base class to follow the Rails idiom.

The main goal here is to keep a good public API so the user will use:

ActiveModel::Serializer::Base # When creating a AR Serializer
ActiveModel::Serializer::Model # When creating a PORO Serializer
ActiveModel::Serializer.logger # To define the logger

I want to hear from you guys, your thought about it? You are 👍 or 👎

[beauby]

Assigning @bf4 to this one as I believe you made those 2 PRs (AMS::Model and the logger). Could you explain the rationale behind exposing those via ActiveModelSerializers rather than ActiveModel::Serializer?

[bf4]

Basically, ActiveModel::Serializer is a terrible name for what it is and I'd like to move away from it. It's both the gem name, a namespace, and a resource attribute mapper.

Also, using a class as a namespace for unrelated things isn't very Ruby-friendly.

I can write more, but does that make sense?

[beauby]

@bf4 I get what you're saying. Maybe it's still worth going into more details here so that we can keep a written trace of the exact reasons why.

[bf4]

I'm going to try to write up something tonight that is json api focused that higkights the mismatching of terms

But also, activemodel:: seializers right now attracts data and behavior not related to it being a serializer class since it is also a namespace that sounds like the namenof the gem
B mobile phone

On Oct 25, 2015, at 4:34 PM, Lucas Hosseini notifications@github.com wrote:

@bf4 I get what you're saying. Maybe it's still worth going into more details here so that we can keep a written trace of the exact reasons why.

—
Reply to this email directly or view it on GitHub.

[maurogeorge]

Thanks for the comments guys.

@bf4 I want to hear more about your idea, I am working a little time on this project so maybe I miss something. Could you please expose what is the idea of the public APIs, something like:

ActiveModel::Serializer::Base # When creating a AR Serializer
ActiveModel::Serializer::Model # When creating a PORO Serializer
ActiveModel::Serializer.logger # To define the logger

The actual state I think is a little confusing having something in ActiveModelSerializers and something in ActiveModel::Serializer as a public API. My focus here is only in the puclic API, I think if we have a goal to a public API we can work on the internals to reach this goal.

[bf4]

Welcome to team 'make things less confusing'

I made a pr about this last night

B mobile phone

On Oct 26, 2015, at 6:59 AM, Mauro George notifications@github.com wrote:

Thanks for the comments guys.

@bf4 I want to hear more about your idea, I am working a little time on this project so maybe I miss something. Could you please expose what is the idea of the public APIs, something like:

ActiveModel::Serializer::Base # When creating a AR Serializer
ActiveModel::Serializer::Model # When creating a PORO Serializer
ActiveModel::Serializer.logger # To define the logger
The actual state I think is a little confusing having something in ActiveModelSerializers and something in ActiveModel::Serializer as a public API. My focus here is only in the puclic API, I think if we have a goal to a public API we can work on the internals to reach this goal.

—
Reply to this email directly or view it on GitHub.

[maurogeorge]

@bf4 great work thanks 👍

Adding a link here to PR #1301 to keep the reference.

[maurogeorge]

@bf4 I think you PR do not cover the ActiveModelSerializer.logger and ActiveModelSerializer::Model since this is not the objective in fact of the PR. But following the comment here #1301 (comment) I think the main entry point and public API will be ActiveModelSerializers right? Correct me if I am wrong.

What I feel a little confusion is about the main entry point, If you take a look at ActiveModelSerializers and ActiveModel::Serializer seens that both are fighting to reach the main entry point. If we take a look at shoulda-matchers or Devise it is clear who is the entry point and how to extend the project.

Just a note, today the gem name is a little confusing too, if we follow the rubygens recomendation, I think the goal was

Gem name	Require statement	Main class or module
ActiveModel::Serializers	active_model/serializers	ActiveModel::Serializers

Based on the bump of 0.10.0.pre released by Klabnik if we take a look on README and gemspec always is used ActiveModel::Serializers.

I think we need to expose only a single namespace ActiveModelSerializer, ActiveModel::Serializer or ActiveModel::Serializers this makes sense to you? I know that is a big change, but my idea here is to clarify the things only.

[bf4]

@maurogeorge Thanks for your thoughts here. I really appreciate it.

So, I haven't thought it all the way through, and I just wrote it out without any review, but what I'm thinking right now is something like:

Current	Desired	Notes
require 'active_model_serializers'	require 'active_model_serializers'
ActiveModelSerializers	ActiveModelSerializers
ActiveModel::SerializableResource	ActiveModel::SerializableResource
namespace ActiveModel::Serializer	ActiveModelSerializers::Current, ActiveModelSerializers::V08, etc	versioned
class ActiveModel::Serializer	ActiveModelSerializers::Current::Resource	is a class that defines the properties and behavior of the data that you present to the user
ActiveModelSerializers::Model	ActiveModelSerializers::Model	e.g. ActiveRecord object but a PORO. Maybe should be ActiveModelSerializers::Record A record is an instance of a model that contains data loaded from a server. Your application can also create new records and save them back to the server.
ActiveModel::Serializer::Lint	ActiveModelSerializers::Model::Lint
ActiveModel::Serialization	ActiveModelSerializers::ActionController::Serialization
ActiveModel::Serializer::CollectionSerializer	ActiveModelSerializers::Current::ResourceCollection
namespace ActiveModel::Serializer::Adapter	ActiveModelSerializers::Adapter	adapter: knows how to talk to your server. knows how to translate requests from AMS into requests on your server. object that translates requests from AMS/Rails/Grape etc (such as "find the user with an ID of 123") into a requests to a server. let you completely change how your API is implemented without impacting your application code.
class ActiveModel::Serializer::Adapter::JsonApi	ActiveModelSerializers::Adapter::JsonApi, ActiveModelSerializers::Adapter::V08::Json	not versioned
class ActiveModel::Serializer, ActiveModel::Serializer::Adapter::Attributes	ActiveModelSerializers::Formatter	maps keys and values to desired format, see https://github.com/orbitjs/orbit.js/blob/master/lib/orbit-common/jsonapi-serializer.js
N/A	ActiveModelSerializers::Current::Store	store is the central repository of records in your application, use the store to retrieve records, as well to create new ones. The store will automatically cache records for you.
N/A	JsonApi::Exceptions
ActiveModel::Serializer::Adapter ::JsonApi::ApiObject::JsonApi	ActiveModelSerializers::Adapter::JsonApi::ApiObject::{JsonApi,Links,Link,Resource,Error, etc}? vs. JsonApiFormat https://github.com/endpoints/endpoints/blob/master/es5/format-jsonapi/index.js or even schema https://github.com/orbitjs/orbit.js/blob/master/lib/orbit-common/schema.js https://github.com/endpoints/endpoints/blob/master/es5/validate-json-schema/index.js
ActiveModelSerializers.logger	ActiveModelSerializers.logger

see

• http://guides.emberjs.com/v2.1.0/models/
• https://github.com/cerebris/jsonapi-resources/blob/master/lib/jsonapi/formatter.rb
• https://andycrum.github.io/ember-data-model-maker/
• https://github.com/cerebris/jsonapi-resources#jsonapiresource
• origins of naming in AMS:
  • rails/rails@d2b78b3#diff-80d5beeced9bdc24ca2b04a201543bdd
  • rails/rails@783db25
  • rails/rails@c6bc8e6
  • rails/rails@fbdf706#diff-531b52ff1432c7c9f9472188b39d6a43
  • rails/rails@4bfbdc1
  • rails/rails@8896b4f
  • https://github.com/rails-api/active_model_serializers/commits/master?page=19
  • 729a823#diff-fe7aa2941c19a41ccea6e52940d84016
  • 4a2d985
  • f00fe55

[bf4]

@maurogeorge would you be interested in turning this issue into a PR so we can comment on diffs?

using the hub gem would be something like

hub pull-request -i 1298 -b rails-api:master -h maurogeorge:https://github.com/maurogeorge/active_model_serializers/tree/rfcs/namespacing

[maurogeorge]

@bf4 really happy that you appreciate my thoughts ❤️

Thanks for the feedback and the idea, first I will work on the #1291 and later I will create the PR.

[maurogeorge]

@bf4 I created the RFC here #1310.

I am closing this in favor of the PR.