Support open ended scalars

[kdawgwilk]

This adds support for defining open_ended scalars that can recieve arbitrary GQL data. We opted to put this behavior behind a flag so that it is an opt-in feature. This was a necessary feature to support the Federation spec's _Any scalar

[kdawgwilk]

Example implementations of the Federation _Any type that is used as a scalar object

• https://github.com/apollographql/federation-jvm/blob/master/graphql-java-support/src/main/java/com/apollographql/federation/graphqljava/_Any.java
• https://github.com/preply/graphene-federation/blob/master/graphene_federation/types.py
• https://github.com/Gusto/apollo-federation-ruby/blob/master/lib/apollo-federation/any.rb

All of them appear to just forward the input object type through and don't really do any coercion so I would expect absinthe to also do something similar

[benwilson512]

As a note for others reading the PR: Much as Absinthe has resisted this feature over the years, it isn't a fight we're gonna win. I think it's time to try to support this.

[binaryseed]

We have an implementation of this in our API with an allow-list for scalars...

So one question is how we want this to be enabled - will it be on by default? opt-in per scalar? something else?

[binaryseed]

We needed to modify:

• Absinthe.Phase.Document.Arguments.Data
• Absinthe.Phase.Document.Arguments.FlagInvalid
• Absinthe.Phase.Document.Arguments.Parse

[kdawgwilk]

We have an implementation of this in our API with an allow-list for scalars...

@binaryseed can you point me in the right direction to look into this implementation? Was it previous PR that didnt get merged? Just realized you meant in your internal API this was implemented.

[kdawgwilk]

Which approach do we want to take on this?

1. create fake schema nodes in the Phase.Schema for Input.Fields when we are unable to find an existing schema node?
   changes would like be here

   absinthe/lib/absinthe/phase/schema.ex

   Lines 165 to 173 in bcfd47f

   	defp set_schema_node(%Blueprint.Input.Field{} = node, parent, schema, adapter) do
   	case node.name do
   	"__" <> _ ->
   	%{node | schema_node: nil}
   	name ->
   	%{node | schema_node: find_schema_field(parent.schema_node, name, schema, adapter)}
   	end
   	end

   to do a find_or_create_schema_field
2. handle Document.Arguments with schema_node: nil by not marking them as invalid anymore, changes would likely be made here

   absinthe/lib/absinthe/phase/document/arguments/flag_invalid.ex

   Lines 21 to 23 in bcfd47f

   	defp handle_node(%{schema_node: nil, flags: %{}} = node) do
   	node |> flag_invalid(:extra)
   	end

   
   I am leaning towards option 2 since generating fake schema_nodes seems like a bad idea

[benwilson512]

I think maybe an option 3, where you pattern match for %{schema_node: %Type.Scalar{}} = node and then return {:halt, node} which will prevent further descent in the tree.

[kdawgwilk]

@benwilson512 The FlagInvalid phase is a postwalk so halting wouldn't help us right?

[kdawgwilk]

Alright I am to a place where I am feeling pretty good about the implementation now and would love some more thorough reviews

[benwilson512]

I'm mostly happy with this, but we need to review spec behavior for scalars that don't accept struct literals, and we need a number of additional test cases to more comprehensively cover the new functionality.

[binaryseed]

This may require that the scalars have some sort of explicit flag.

Agreed, I think this needs to be opt-in only, and per-scalar. An API will want both Scalars that are open-ended and some that aren't. Out of the box, the addition of this feature should not change any existing behavior.

[kdawgwilk]

Any ideas on what we want the opt-in API to look like?

scalar :any do
  meta :dynamic, true
  ...
end

[binaryseed]

I think we may need a new syntax / field, since we should probably leave meta for usage by the end user & not Absinthe.

I tend to like calling these "open ended", what do you think @benwilson512

[benwilson512]

@binaryseed agreed, I'd be fine with like:

scalar :any, open_ended: true do
  # stuff
end

[kdawgwilk]

I am happy to make more changes but I think I have addressed the feedback so far

[kdawgwilk]

Is this something that would make sense to live in absinthe? Or would it be better as some sort of plugin to absinthe? I need this feature for my use case of federation but I may be able to put this behavior into the absinthe_federation library itself if we dont want this to be a core feature of absinthe. Would love some direction on this so I can move forward with whatever the team thinks is best for the absinthe ecosystem

[benwilson512]

The test cases look largely correct to me now, I'll review in more detail later this week. I think supporting it this way in Absinthe is important since it needed changes to key files.

[kdawgwilk]

Once this merges can we release it as an RC? I would like to get the absinthe_federation library on hex but it will depend on these changes. I would also like to submit a talk proposal to Elixir conf to talk about federation and those submissions are due July 11th, 2021

[benwilson512]

These changes look good! Will look at doing a release shortly.

[dylan-chong]

Thanks so much for this PR @kdawgwilk !

@benwilson512 At Solve, we were looking at using this feature for a long time. I've got an open PR to use this new feature, but I just wanted to double check if this is a feature that will continue to be supported

[benwilson512]

Yes, this will be supported.

[razielgn]

Was there a specific reason for not supporting open ended scalars that are lists? I have data about fixed-sized arrays (like Vec3). Having list_of(:float) feels too generic, while handling their validation at schema level, plus providing exact documentation using a scalar would be very useful.
If it doesn't go against GraphQL's spec, I could make an attempt at submitting a PR.