node and nodes scalar type configuration

[crpahl]

PR Description

This PR makes it possible to configure the node and nodes scalar type through the Configuration class.

For example, you could do this in a Rails initializer:

GraphQL::Configuration.relay_node_id_type = "SomeScalar"

And the arguments to node and nodes would be id: SomeScalar! and ids: [SomeScalar!]! respectively. The returned id field would be id: SomeScalar!.

Why is this needed?

We don't use the GraphQL Ruby ID scalar and instead use our own scalar for ID fields. Maybe we should consider going back to the ID scalar, but that's a different discussion/concern for if we ever want to go back in that direction 😄 This PR makes it possible to use our own scalar for the root node and nodes field.

GraphQL Ruby doesn't support this type of configuration?

I ultimately need to be able to set the scalar type at Ruby load time. Here for example:

child_module.field(:id, Configuration.relay_node_id_type, null: false, description: "ID of the object.", resolver_method: :default_global_id)

And setting this in a configuration class was the easiest way for me to do this through a simple Rails initializer. I realize this isn't a pattern that GraphQL Ruby takes for configuration (unless I missed it!) and I'm open to any other approaches!

Other approaches we considered

I realize I could also completely override the modules, but it felt much better to be able to configure/inject the type and save us from awkward monkey patching.

[crpahl]

@rmosolgo I wanted to throw this proof of concept together quickly to get your thoughts 🙏 I'm open to any changes you suggest, or whatever you feel makes sense!

[crpahl]

Hmm after looking closer at this, maybe the configuration of the id field and argument type should go somewhere in our base connection class with a new id_type class method?

[rmosolgo]

I'm open to accepting a customization here, but I don't want a global configuration object. Lots of applications run multiple schemas and I want to make sure that they can be configured separately. Here are a few other APIs I could imagine:

1. Mixin plus configuration method:

   module HasCustomNodeField 
     extend HasNodeField 
     id_type(Types::CustomID) # your scalar here
   end
   # Something similar for HasCustomNodesField 

2. Generated module:

   # `HasNodeField.[]` returns a generated module:
   include HasNodeField[id_type: Types::CustomID]
   include HasNodesField[id_type: Types::CustomID]

3. Inject via method params:

   # Instead of `include ...`:
   HasNodeField.add_field(self, id_type: Types::CustomID)

I'm definitely game to support something like this -- is there one of of those alternatives (or another non-global one) that you'd like to explore?

[crpahl]

I like Approach 2! It's new to me and I took a quick stab at it. Is this what you had in mind? I implemented it in a way that both:

• include GraphQL::Types::Relay::HasNodeField
• include GraphQL::Types::Relay::HasNodeField[id_type: <some_type>]

will work so it won't be a breaking change. I'd still need to inject it here into the Node interface in some way:

child_module.field(:id, ID, null: false, description: "ID of the object.", resolver_method: :default_global_id)

Do you have suggestions for that too? 🙏

[benzittlau]

@crpahl How many different variations of the ID scalar are we anticipating to be needed? I'm not familiar with the context around how this is effecting the path towards relay compliance, so maybe you could give me some quick context or point me towards some references.

[crpahl]

How many different variations of the ID scalar are we anticipating to be needed?

Hey @benzittlau, just one.

For context, we use our own custom ID scalar. If we could travel back in time, we probably would have just used the ID scalar 😆

So that made me think of two options to work towards becoming Relay compliant:

• A PR like this that allows us to use our own custom ID scalar for object identification
• A large breaking change set that updates all of our custom ID scalars back to true ID scalars

[benzittlau]

@crpahl

I want to make sure I understand the context fully so am going to use this as an opportunity to do some digging into our current GraphQL implementation (in particular around ID handling). I'll come back once I have more understanding to make sure it aligns and we can go from there.

While I'm doing that, you might be able to address one of the questions/concerns I have with the current implementation in your PR. Ruby meta programming is a "here be dragons" risk as it can be very hard to trace and debug (and for non-Ruby experts understand). The dynamic module approach in the PR is really nifty, but if there is a limited set of ID scalars we need, why not just explicit define them individually.

Instead of:

include GraphQL::Types::Relay::HasNodeField[id_type: IDTypeA]
include GraphQL::Types::Relay::HasNodeField[id_type: IDTypeB]

Do:

include GraphQL::Types::Relay::HasNodeFieldIDTypeA
include GraphQL::Types::Relay::HasNodeFieldIDTypeB

[crpahl]

Yeah that's fair @benzittlau! I think after syncing up on this, we'll go with your other suggestion for now and revisit something like my PR later if really needed.