Make a nice API for composition

[rmosolgo]

What if you have a bunch of types with the same fields? I've been passing the objects into functions and adding fields to them, but it would be good to have a proper API for that. One idea I have is a "mixin", like:

HasBirthday = GraphQL::Mixin.define do 
  field(:birthday, DateType)
  field(:age_in_years, types.Int)
  field(:can_vote, types.Boolean) 
end

Person = GraphQL::ObjectType.define do 
  mixin(HasBirthday)
end

Cat = GraphQL::ObjectType.define do 
  mixin(HasBirthday)
end

[peterhorne]

Perhaps this is because a simple example has been used for pedagogical purposes but it seems to me that types currently support this scenario just fine. Person and Cat both have a birthday field which resolves to a Birthday type with fields date, age_in_years, and can_vote?

That said I have run into scenarios where it would be useful to be able to define resolve methods on an interface's fields (perhaps that can be optionally overridden by the implementing type). Would that cover the use cases supported by mixins while also keeping the number of concepts small?

[rmosolgo]

That's an interesting point! Use "subtypes" to capture that shared behavior. I share your desire to keep the number of concepts small. Maybe a suitable solution to this issue would be to add a guide for "Code Reuse Patterns"

That example is a bit contrived, here's a real-life example:

With "mixin"

HasCheckInCounts = GraphQL::Mixin.define do 
  field(:total_count, types.Int) 
  field(:guest_count, types.Int) 
  field(:regular_count, types.Int) 
  field(:volunteer_count, types.Int) 
end

EventType = GraphQL::ObjectType.define do 
  mixin(HasCheckInCounts)
end 

LocationType = GraphQL::ObjectType.define do 
  mixin(HasCheckInCounts)
end 

With a module

This is how I've been solving problems like this lately. There's nothing pretty about it but I like it because it uses plain-Ruby concepts and the API gives plenty of room for customizing or changing the implementation later on.

module CheckInCountsFieldsFactory
  COUNT_FIELDS = [
    :total_count,
    :guest_count,
    :regular_count,
    :volunteer_count,
  ]

  def self.create(object_type)
    COUNT_FIELDS.each do |field_name|
      object_type.field(field_name, object_type.types.Int)
    end
  end
end

EventType = GraphQL::ObjectType.define do 
  CheckInCountFieldsFactory.create(self)
end

LocationType = GraphQL::ObjectType.define do 
  CheckInCountFieldsFactory.create(self)
end

With a type

This solution is so nice because it uses GraphQL basics. I like the flexibility provided by the resolve proc. In these cases, I'm just passing along the underlying obj, but you could do whatever you want there: fetch a new object, create a proxy object, whatever!

The only thing I don't like is how the GraphQL structure diverges from the ActiveRecord structure. That is, fields that used to be on EventType now belong to a nested CheckInCountsType object.

CheckInCountsType = GraphQL::ObjectType.define do 
  field(:total_count, types.Int) 
  field(:guest_count, types.Int) 
  field(:regular_count, types.Int) 
  field(:volunteer_count, types.Int) 
end

EventType = GraphQL::ObjectType.define do 
  field(:check_in_counts, CheckInCountsType) do 
    # pass in the Event itself, since the methods are all on the Event 
    resolve -> (obj, args, ctx) { obj } 
  end
end

LocationType = GraphQL::ObjectType.define do 
  field(:check_in_counts, CheckInCountsType) do 
    resolve -> (obj, args, ctx) { obj } 
  end
end

[charlieschwabacher]

This would be a big breaking change, but what if types were classes instead of instances, w/ class methods used for definition. That way we could do something like:

module CheckInCounts
  extend ActiveSupport::Concern
  included do
    field :total_count, types.Int
    field :guest_count, types.Int
    field :regular_count, types.Int
    field :volunteer_count, types.Int
  end
end

class EventType < GraphQL::ObjectType
  include CheckInCounts
  field :name, types.String
  field :date, types.Int
end

class LocationType < GraphQL::ObjectType
  include CheckInCounts
  field :name, types.String
  field :address, AddressType
end

and could use inheritance like:

class VolunteerType < UserType
  field :numberOfHoursVolunteered, types.Int
end

class GuestType < UserType
  field :guestOf, UserType
end

Is there some reasoning behind DefinedByConfig that makes it necessary vs something like this?

An added benefit (actually the reason I started thinking about this) is that we could break long type specific field definitions into their own classes under the namespace of a type. I have been ending up w/ some 30-40 line resolve procs in complex types that feel like they deserve their own files and unit tests - I think something like this could make that easier.

# type w/ long resolve 
class EventType::Guests < GraphQL::Field
  type types[UserType]
  argument :order, types.String
  argument :role, types[UserRolesEnumType]
  argument :minNumberOfEventsAttended, types.Int

  # complex code specific to arguments and context goes here 
  def resolve(obj, args, ctx)
    # ....
  end
end

class EventType < GraphQL::ObjectType
  field :guests, field: EventType::Guests.new
end

[rmosolgo]

I definitely agree that we need some clear paths for breaking up large type definitions!

Configuration like this is partly supported:

field :guests, field: EventGuestsField 

I say partly because I think the incoming field is mutated during the field call, so it might not work to reuse a field in multiple definitions. Instead of class, you would use GraphQL::Field.define. It would be great to implement that so it didn't mutate its argument, that would be a lot better!

Types as Classes?

Before the GraphQL spec was released (and there was only some passing mention of the idea from ReactConf), I worked up an "implementation" where types were classes which "wrapped" the object they exposed.

I guess it wasn't just the class-driven aspect of that structure that seemed bad, but it had lots of problems:

• Type classes were overloaded -- they were responsible for defining themselves, resolving themselves and resolving their own fields (recursively). The base class was a mess.
• The delegation behavior was an accident waiting to happen. What if you had a field whose name conflicted with an important method on the parent class? I've seen Rails code for coping with that, ick!
• What's the point of initialize in that case? The Type is really just a big configuration object, so making instances of it didn't really make sense. Once it's configured, the data never changed.
• Testing was not pretty. To get the full picture, you really needed an instance of the Type object, which is normally a private concern of the gem code. So I wrote some test helpers but ... I thought it would be better to aim for a system that didn't need test helpers!

Anyways, that's why I departed from the class-based type system. Maybe we did lose something though, since we're so used to sharing behaviors through inheritance!

[xuorig]

I like how your mixin solution looks @rmosolgo. In you CheckIn example, couldn't that be solved using an Interface like Countable ? I guess it's when you need different interfaces that mixins could come handy.

Is this something that could potentially be solved by graphql/graphql-spec#48 ?

[rmosolgo]

Interface is nice for publishing your API, but AFAIK it doesn't actually implement fields for you. You still have to say field :whatever in the ObjectType. But I think that's ok: two ObjectTypes might expose the same fieldnames but resolve them differently.

[peterhorne]

The issues I have with mixins rather than default implementations for interfaces are:

• They don't provide any additional functionality
• Useful information for the client is thrown away
• Given the above, the cost of introducing a new concept for no benefit

In detail:

Interfaces being able to provide a default implementation is functionally equivalent to mixins and therefore can support all the same use cases. The disadvantage of mixins is that while your type now conforms to an implicit interface, that is not communicated through the graph so useful information is being thrown away.

To consider your original example HasBirthday example: it is not possible to define a Relay component for rendering attributes of the HasBirthday mixin across generic types unless you introduce a corresponding HasBirthday interface.

You won't always need to have the interface of a mixin exposed in the graph but it seems a shame to throw away potentially useful information that could have been available for free.

Regarding your comment above: Two types exposing the same fields conforming to an interface, but resolving differently, can be handled if an interface's default resolves can be overridden in implementing types. I believe this is how interfaces work in graphql-js.

[rmosolgo]

All good points about interfaces with default implementations, you get the benefit of Mixin and then some.

[rmosolgo]

Thanks for the great discussion, I merged a try at using interfaces as mixins #108