graphql / graphql-spec

GraphQL is a query language and execution engine tied to any backend service.
https://spec.graphql.org
14.26k stars 1.12k forks source link

Proposal: Support union scalar types #215

Open stubailo opened 7 years ago

stubailo commented 7 years ago

It makes sense often to have a field that could return, for example, an Int or a String. It would be convenient to be able to use a resolveType-like mechanism to determine how the value is serialized. This would be even more useful for custom scalars.

The specification currently says: "GraphQL Unions represent an object that could be one of a list of GraphQL Object types", which basically means that unions are the same as interfaces, but without any shared fields. Supporting unions of scalars would further differentiate unions from interfaces.

rmosolgo commented 7 years ago

Could you share a couple of concrete use cases? I ask because, personally, I'm usually reading values right out of MySQL (or running those values through transformations with predictably-typed output), so I haven't needed this yet!

migueloller commented 7 years ago

@rmosolgo,

Here is a concrete use case that we've come across at my team. I have a couple more if you need them.

We've got an algorithm that produces results based on location. We have an enum AreaOfTown that contains all possible areas of town to filter the results. Here's the catch, the algorithm can also filter locations by "near me". Because we can't simply make a union that is AreaOfTown | "NEAR_ME" for the input type, then we have to create a completely new enum that contains all of the values in AreaOfTown and "NEAR_ME". This is undesirable because if we ever change the AreaOfTown enum by adding more (this will 100% happen in the future), then we have to make sure we also update our other enum.

If we could write scalar unions then it would solve this problem for us, among others (we would like to have constructs like Int | [Int!] or Int | String like @stubailo mentioned).

stubailo commented 7 years ago

In our case, it's because we were returning a structure that represented a JSON tree, where keys can be integers (in the case of arrays) or strings (in the case of objects). We ended up compromising for now to just use strings, but I can imagine how this would be extra useful in the case where you want to return one of a set of custom scalars, which might have different serialization logic.

migueloller commented 7 years ago

Here are other relevant discussions:

@leebyron made some good points in his comments here.

marcintustin commented 7 years ago

Allowing uniontypes to contain primitives allows us to model any kind of source that can be polymorphic in its source data. For example, avro data or elasticsearch.

This is a severe limitation on exposing existing data, rather than creating new systems that are defined by graphql from the ground up.

leebyron commented 7 years ago

What's still unclear to me is what a valid query would look like which encounters such a union, and how should well-typed clients determine what to expect from such a query?

Consider:

type MyType {
  field: String
}

union MyUnion = String | MyType

type Query {
  example: MyUnion
}
marcintustin commented 7 years ago

Like with any union - it has to be prepared for any item in the union. I don't see that this poses any special problems.

On Wed, Feb 8, 2017 at 7:52 PM Lee Byron notifications@github.com wrote:

What's still unclear to me is what a valid query would look like which encounters such a union, and how should well-typed clients determine what to expect from such a query?

Consider:

type MyType { field: String } union MyUnion = String | MyType type Query { example: MyUnion }

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/facebook/graphql/issues/215#issuecomment-278512267, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLqONw9c-XUFzYcBYSTtPU2GGBsr5J7ks5ramNMgaJpZM4KLagd .

leebyron commented 7 years ago

For the above schema consider the query:

{ example }

It's clear what this query should return if the example field returns a String at runtime, but what should it return if the value was a MyType?

Similarly:

{ 
  example {
    ... on MyType {
      field
    }
  }
}

It's clear what this would return for MyType, but what should it return for a String?

marcintustin commented 7 years ago

The exact same thing as querying a value that doesn't exist on a member of a union.

On Wed, Feb 8, 2017 at 8:36 PM Lee Byron notifications@github.com wrote:

For the above schema consider the query:

{ example }

It's clear what this query should return if the example field returns a String at runtime, but what should it return if the value was a MyType?

Similarly:

{ example { ... on MyType { field } } }

It's clear what this would return for MyType, but what should it return for a String?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/facebook/graphql/issues/215#issuecomment-278520186, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLqONSR3whAYxVVrPBjdCmGOT_Py57tks5ram2rgaJpZM4KLagd .

leebyron commented 7 years ago

It's not clear to me what that means. What would a valid query look like that would clearly give a value for a primitive type or an object type?

leebyron commented 7 years ago

Specifically http://facebook.github.io/graphql/#sec-Leaf-Field-Selections is what I'm referring to as "valid query"

stubailo commented 7 years ago

I think the original issue is about having unions where every member is a scalar right? Not unions between scalers and objects?

OlegIlyenko commented 7 years ago

just an idea. Maybe it would make sense to introduce special inline fragment semantics for scalars:

{ 
  example {
    ... on String {
      value
    }
    ... on MyType {
      field
    }
  }
}

This can be generalized by allowing scalars to be used as leafs (current semantics) and as objects with a single field (value). A huge advantage of this approach is that one can use __typename introspection on scalars.

In our API we have a similar issue. For some parts of the schema, we even have introduced object types like StringAttribute, IntAttribute, etc. in order to unify object attributes ("attributes" are a part of user-defined data structures which our service supports) and scalar attributes and provide same introspection capabilities for both.

migueloller commented 7 years ago

What if the result was a scalar then the result would simply ignore the selection set, and if it is an object type then it would consider the selection set. Statically, the query can be analyzed because the schema should have enough information to know if the union contains a scalar or an object or both.

If there is no selection set and the value is an object then the returned value can just be {}. If there is a selection set and the value is a scalar then the returned value can just be the scalar value (ignoring the selection set, just like it is done for object type unions).

On the client most type system allow unions of arbitrary types, so the client can expect a result that is string | { name: string } in Flow, for example.

Nevertheless, this starts getting into the design choices of the simplicity of GraphQL, which I think @leebyron made great points here: https://github.com/graphql/graphql-js/issues/207#issuecomment-243638767

calebmer commented 7 years ago

Mixing scalars and object types would require a change to the schema which is too complex to consider. Schemaless GraphQL clients depend on the guarantee the { field } is scalar and { field { a b c } } is composite. Given that a scalar field could be an object this is the only reliable way to tell a scalar apart from a composite value without looking at the schema. By breaking that guarantee you introduce a lot of unnecessary complexity into the GraphQL client when you could easily just add a composite StringBox type or similar:

type StringBox {
  value: String
}
migueloller commented 7 years ago

@calebmer, agreed! It's a balance between feature-set and simplicity and I think the GraphQL designers wanted to err on the side of simplicity. Please correct me if I'm wrong @leebyron.

Given this, I think that the original discussion of scalar-scalar (or more specifically, leaf nodes) unions is what we should perhaps consider (as opposed to object-scalar unions).

rmosolgo commented 7 years ago

It seems like the Box approach could solve most of these issues without any changes to the spec, right?

# These two fields are mutually exclusive:
type AreaOfTownBox {
  stringValue: String 
  enumValue: AreaOfTown
}

# or 
type IntOrStringBox {
  stringValue: String
  intValue: Int 
}

It pushes the complexity to the schema but keeps GraphQL simple and predictable. Are there downsides to that approach other than verbosity?

migueloller commented 7 years ago

@rmosolgo, you are right that it is possible but it comes at the cost of added complexity in the schema, verbosity in the queries, and having to check each value in the client side at runtime to determine which field was returned.

calebmer commented 7 years ago

Another approach inspired by @rmosolgo’s recommendation would to have a type system that looks like:

union IntOrString = IntBox | StringBox

type IntBox {
  value: Int
}

type StringBox {
  value: String
}

With a query that looks like:

fragment intOrString on IntOrString {
  __typename
  ... on IntBox { intValue: value }
  ... on StringBox { stringValue: value }
}
IvanGoncharov commented 7 years ago

Because we can't simply make a union that is AreaOfTown | "NEAR_ME" for the input type, then we have to create a completely new enum that contains all of the values in AreaOfTown and "NEAR_ME". This is undesirable because if we ever change the AreaOfTown enum by adding more (this will 100% happen in the future), then we have to make sure we also update our other enum.

@migueloller This problem can be solved directly in your code without any change to GraphQL. Wehn you define your types in source code just create a small utility function like extendEnumValues in the example below:

new GraphQLEnumType({
  name: 'NewName',
  values: extendEnumValues(AreaOfTown.getValues(), ['NEAR_ME']);
});

As for IDL, I think it would be great to have support for it there, for example allowing extend to be used on enums. But this is a topic for separate issue/PR.

Allowing uniontypes to contain primitives allows us to model any kind of source that can be polymorphic in its source data. For example, avro data or elasticsearch. This is a severe limitation on exposing existing data, rather than creating new systems that are defined by graphql from the ground up.

@marcintustin There are number common JSON patterns that you can't express in GraphQL: key-value pairs (aka Maps), Tuples, etc. I believe this is a bad idea to make GraphQL a superset of all popular formats/protocols.

Given this, I think that the original discussion of scalar-scalar (or more specifically, leaf-nodes) unions is what we should perhaps consider (as opposed to object-scalar unions).

@migueloller There are many cases when API client can't detect type based on the value returned. For example, union of enums having the same value. It is pretty common for enums to have common values like None, Undefined, etc. Another example is a union of custom scalars with the same base type like AbsoluteUrl|RelativeUrl.

Also, API client should explicitly specify types which he can handle using ... on TypeName { construction. Without it, if you change Number|Boolean to Number|Boolean|String it will break client which don't expect the value to be a string.

That mean you can't serialize union of scalar types as a scalar value and forced to support some equivalents of __typename and ... on ScalarType { constructs. So the end result will look very similar to solution proposed by @calebmer

Personally, I think unions of scalar types are frequently abused. A common example of this is returning an array of strings or the single string only because array with one element looks not so nice :) It forces all API clients to add few more lines of code for every instance of such anti-pattern.

For the cases when you return arbitrary data you can always fall-back to providing value as a JSON string. Such pattern is already used by GraphQL introspection to return defaultValue http://facebook.github.io/graphql/#sec-The-__InputValue-Type

Also, in SQL, you can have only one type per column and it doesn't prevent it from being dominate language for databases.

ianks commented 7 years ago

A common example of this is returning an array of strings or the single string only because array with one element looks not so nice

There are legitimate use cases that are not simply cosmetic, however.

Here is an example from the Facebook Marketing API. When attempting to model filtering in the API, you have an object that represents a filter expression, like so:

filtering: [{ field: "clicks", operator: "LESS_THAN", value: 42 }]

However, value is by nature polymorphic, as it has to support expressions like this, as well:

filtering: [{ field: "campaign_id", operator: "IN", value: ["1", "2", ...] }]

Currently, this is not possible to express safely AFAIK. Scalar-union types would make this implementation trivial. Falling back to a JSON string for this seems like a poor solution. Some downsides:

  1. validate against the schema
  2. interactive tools like graphiql become less useful
  3. introducing another data serialization format into your resolvers, which can be a point of failure
  4. lack of type safety

I believe this is a bad idea to make GraphQL a superset of all popular formats/protocols.

This is a strawman. The argument does not necessitate GQL to be a superset of all protocols. Adding scalar union inputs certainly would not make GQL a superset of all protocols.


It seems like the Box approach could solve most of these issues without any changes to the spec, right?

# These two fields are mutually exclusive:
type AreaOfTownBox {
  stringValue: String 
  enumValue: AreaOfTown
}

# or 
type IntOrStringBox {
  stringValue: String
  intValue: Int 
}

@rmosolgo The problem I see with this is you assume that the two fields are mutually exclusive; however, there is no way to statically define that in the current type system.

IvanGoncharov commented 7 years ago

A common example of this is RETURNING an array of strings or the single string only because array with one element looks not so nice

@ianks The entire issue is about returning data, it was clearly stated in the initial comment

It makes sense often to have a field that could RETURN, for example, an Int or a String

It has nothing to do with input objects since they are not only missing unions of scalars but don't support unions at all. If you feel that supporting unions of types (including scalars) is essential for GraphQL please open a separate issue.

Here is an example from the Facebook Marketing API

In this example, you know all possible fields in advance, moreover, you know which filtering is possible on which field. So you can represent this in terms of the current GraphQL spec like below:

filtering: {
  clicks: {
    LESS_THAN: 42 
  },
  campaign_id: {
    IN: ["1", "2"]
  }
}

Advantages of this are much better autocompletion and validation on the client side than when using unions of scalars.

calebmer commented 7 years ago

@ianks here’s some related issues on union input types: https://github.com/facebook/graphql/issues/202 and https://github.com/graphql/graphql-js/issues/207

RomanHotsiy commented 7 years ago

@calebmer

Another approach inspired by @rmosolgo’s recommendation would to have a type system that looks like:

union IntOrString = IntBox | StringBox

type IntBox {
  value: Int
}

type StringBox {
  value: String
}

There is one more alternative approach. You can just use custom scalars. GraphQL doesn't specify how custom scalars should be serialised so you can return anything, even free-form JSON. For example graphql-type-json and corresponding article in Apollo docs Custom scalars lack types info though. But this is not an issue for using them. A notable example is GitHub API that has a number of custom scalars like DateTime, HTML, URI and information about type of those scalars (string) is present ONLY in description.

j0nd0n7 commented 7 years ago

I was trying to do filtering by a single ID or an array of IDs and I get here :-( my case:

union IDFilter = ID | [ID]
friends(id:IDFilter): [Friend]

now I have to go like this: friends(id:ID, ids:[ID]): [Friend]

In my opinion and preference, the first option is better, you always filter using id:xxx

josepot commented 6 years ago

@jdonzet In your case, wouldn't it be better to just do:

friends(idsFilter: [ID!]): [Friend]

If idsFilter is null there is no filter, you return all Friends... If it's not null, the Array must have at least one ID...

acjay commented 6 years ago

I posed a somewhat related question here: https://stackoverflow.com/q/47933512/807674

Although, my question deals with the awkwardness of modeling union variants that should contain no data other than their mere presence (i.e. singletons). I do think it would be extremely helpful to have one blessed syntactic way of approaching this data modeling question, rather than a hodgepodge of ad hoc approaches.

At the end of the day, these all seem to be special cases of the question of "how far GraphQL should go from a data modeling perspective?" I would argue that obvious ways of representing algebraic data types would be extremely helpful. Mostly, this boils down to union allowing singletons (and at that point, may as well allow all scalars too). There's already the concept of an enum, which is really just a union of singletons. There's just no obvious bridge between that and proper union.

More than just a question of syntactic convenience, it means bumpy transitions for data models as they grow in complexity.

JarredMack commented 6 years ago

Just commenting to say I've also got a use case similar what @acjay describes - I'm working on a query which can either resolve a user ID as a string, or optionally go resolve the user data from a remote service.

My first pass was to basically define a custom field resolve which return a resolved user, and just map it in the query - e.g.:

query {
  theResource(id: 'foo') {
    user: resolve {
      id
      name
    }
  }
}

query {
  theResource(id: 'foo') {
    user
  }
}

But ideally I'd like to implement it without needing to specify the custom type, i.e., dropping the resolve from above and either returning immediately as a string, or if additional fields are requested, resolve them.

I'm aware that I could get around this by just having id as part of the returned User object and conditionally resolve the rest, but I'm working with an existing REST API which returns String | User, and I want to mirror the behaviour.

falconmick commented 6 years ago

My use case:

I am building a graphql proxy infront of an API. This Api has an array of component’s data for a given page. Now I have 20 possible components and I would rather not make 20 queries that get all of the fields the API returns just so that I can use revolvers to fetch secondary items like forms. So what I want to do is define a set of components that need to be a part of a union so I can socify what to resolve, however I want a catch all of a custom scalar that basically json wraps all fields returned.

If union types supported this I could specify what components need a type (to be added to the union) and the ones which don’t fetch extra data get caught in the JSON scalar type and all the data is serialised and no further resolvers are called.

I’ll edit this when I get to work, super Zapped but needed a brain dump

GroofyIT commented 6 years ago

the recommendation of @calebmer seems the best fit to me, as a nice-to-have it could be partially build in as a standard. (meaning no boxes have to be created by developers).

The issue at hand is that scalar types are no object types. However as available in many languages a developer can choose to handle a scalar type as an object type or not (consider string vs String in c# for example). Following that perspective, a scalar type can be considered as a "shorthand" for the alternative object type. aka "my string" === { __typename:"string", value:"my string" }.

Within a query the user/developer is the one that makes the choice how to handle it. given:

type MyType {
  field: String
}

union MyUnion = String | MyType

type Query {
  example: MyUnion
  exampleString: String
}

Could allow for:

example {
    ... on String {
      value
    }
    ... on MyType {
      field {
         value
      }
    }
  }

example {
    ... on String 
    ... on MyType {
      field
    }
  }

exampleString 
exampleString {
     value
}

As for my use case:

In a task management system, a task can be supplied with contextual data. Of what this contextual data consists is defined by a task definition, which is created by a user. So it can be anything from a scalar to an object. Say for example a task's definition states it can have contextual data:

When an (frontend) application wants to show the task information it sends over a query. We could just send over the client id and let the frontend look it up in another query, but that's not really harnessing the power of graphql. It would be cooler (and more performant) to let the application handle it inline:

task(id:"1234") {
     name
     fields {
        fieldName
        fieldValue {
         ... on String { value }
         ... on Client { id name }
       }
    }
}

There are alternatives into how to implement this of course (dynamic type system/describers/...), though a mixed union of scalar and object types would simplify a lot.

mrdulin commented 6 years ago

My case is the payload field can be Int, or String scalar type. when I write it like union type:

const schema = `
  input QuickReply {
    content_type: String
    title: String
    payload: Int | String
    image_url: String
  }
`

I got an error:

GraphQLError: Syntax Error GraphQL request (45:18) Expected Name, found |

    44:     title: String
    45:     payload: Int | String
                         ^
    46:     image_url: String
mike-marcacci commented 6 years ago

I've been wrestling with something that's related to this, but isn't this exactly. I'm curious if my issue is what's been driving many of these use cases.

My need is not necessarily to have a codified scalar union, but to ergonomically query a heterogeneous list that currently results in a field merge conflict. For example:

type Foo = {
  payload: Number
}

type Bar = {
  payload: String
}

type MyUnion = Foo | Bar;

type Query {
  example: [MyUnion]
}

The current field merging logic prohibits the following query:

query {
  example {
    payload
  } 
}

because payload can be either String or Number.

To get around this, we must either run example twice with conditional fragments (which is incredibly inefficient if the list is large), or alias the payload property so payloadString and payloadNumber in conditional fragments (which is intensely frustrating or impossible when the field list comes from component fragments somewhere downstream).

In order to support scalar unions, the merge rules would have to be loosened, and the above would become valid even without an explicit scalar union.

So I guess I'm curious if the rule change itself would solve the problems of people here. If so, that might be a good separate first step (and I can create an issue if there's interest).

speller commented 5 years ago

Support this proposal. In my project I want to implement a logic when field manufacturer is mentioned in query then the string value will be returned. If client will request manufacturer(id, name) then the object value with fields id and name will be returned.

Kadrian commented 5 years ago

Have a similar real-world example here. A Product has a list of attributes. Each attribute always has a name and a value. In case it's a number attribute, let's say: "weight", there's an additional unit possible, e.g. "kg".

Coming from Gatsby here, this structure won't work during Schema generation, because value has conflicting types. I guess the solution is still that I'd have to do something like valueString, valueNumber, valueBool, valueRange, right?

type ProductType = {
  name: string,
  attributes: AttributeType[]
};

type AttributeType =
  | StringAttributeType
  | NumberAttributeType
  | BoolAttributeType;

type StringAttributeType = {
  name: string,
  value: ?string,
};

type BoolAttributeType = {
  name: string,
  value: ?boolean
};

type NumberAttributeType = {
  name: string,
  value: ?(number | RangeType),
  valueVariance?: number,
  unit?: string, // If no unit, then it's probably a count or similar
};

type RangeType = {
  from: number,
  to: number
};
leebyron commented 5 years ago

Marking this as Strawman and Needs Champion, however I'm still very skeptical since not all target server or client languages support scalar unions.

GraphQL already prevents (via validation rules) other scenarios where a field could return more than one kind of type.

If anyone decides to take ownership of this proposal, I think the bar will be set high to prove value and early attention should be placed on how it would affect code generation systems, especially ones based in Java/C/C++/ObjC.

bmeck commented 5 years ago

Just throwing onto the list of examples, JS heap snapshots have a string_or_number type from V8 at least, and boxing would make the already verbose queries into heaps fairly significant since it would be done at every Edge of a Node.

not-only-code commented 5 years ago

@jdonzet In your case, wouldn't it be better to just do:

friends(idsFilter: [ID!]): [Friend]

If idsFilter is null there is no filter, you return all Friends... If it's not null, the Array must have at least one ID...

But if idsFilter is 343234 will throw an exception because is expecting an Array...

josepot commented 5 years ago

@not-only-code you are missing the point. ~Of course it would throw! Because the signature that I'm proposing only accepts a non-empty Array of IDs or null... You know that you can have an Array with a single item, right?~ Actually, as @benjie pointed out, it wouldn't throw, it would work perfectly fine :smile:.

All I'm saying is that IMO this

friends(idsFilter: [ID!]): [Friend]

Is a better signature than this:

friends(id:ID, ids:[ID]): [Friend]

Because what would be the expected behavior if I passed an ID to the first argument and a list of IDs to the second argument (which doesn't include the ID of the first argument)?

It doesn't matter what the answer to that question is, because whatever it is, it will be an arbitrary decision based on an implementation detail... Which means that signature creates a leaky abstraction. Because you would have to be familiar with an arbitrary implementation detail in order to use it "correctly". Even if your answer is "you can only use one and not the other", that constrain wouldn't be enforced by the signature. So, whichever behavior you define for that signature: it would be an arbitrary one based on an implementation detail. That IMO is something that should be avoided. The signature that I'm proposing -on the other hand- doesn't have that problem.

benjie commented 5 years ago

(Aside: calling friends(idsFilter: 34234) against friends(idsFilter: [ID!]) should not actually throw; see the "Input Coercion" section of TypeSystem > List in the GraphQL spec.)

gregorybellencontre commented 4 years ago

Hello I'm searching for something similar.

I created a resolver to return people list in two possible formats :

And actually, it seems I can't apply a union between String and [People]!

I could make two resolvers : peopleAsString and peopleAsObject but it's really dirty, and I'd rather use arguments to pass the format inside my resolver.

achimnol commented 4 years ago

I'd like to support legacy clients that send String but want to extend support of list of strings for newer clients [String] for a specific query, using String | [String]. If I understand correctly, the GraphQL's design purpose includes "evolvability". Wouldn't this be a case for it?

benjie commented 4 years ago

@gregorybellencontre In the mean time, a sensible solution would be to add a field to a Relay connection object to retrieve the nodes as a string:

type PersonEdge {
  cursor: String
  node: Person
}
type PersonConnection {
  edges: [PersonEdge]
  pageInfo: PageInfo
  asString: String # <<< Add this
}
type Query {
  people: PersonConnection
}
{ people { asString } }
{ people { edges { node { id name } } } }

@achimnol See my comment above, this is already supported by the GraphQL spec's list coercion. No need for the union, just change the type to [String].

tatianafrank commented 4 years ago

Im also trying to define a field of type [String] | String. This is for a UI with filters where there is sometimes a single filter which is a string or an array of strings for multiple filters. Right now the user has to pass an array even if they only have a single filter which is confusing and awkward.

benjie commented 4 years ago

@tatianafrank If you make the field of type [String] then you should be able to pass a String to it ─ see the "Input Coercion" section of TypeSystem > List in the GraphQL spec.

tatianafrank commented 4 years ago

I get an error when I pass a String instead of an array.. Not sure if its being caused by another library Im using..

deser commented 4 years ago

Marking this as Strawman and Needs Champion, however I'm still very skeptical since not all target server or client languages support scalar unions.

GraphQL already prevents (via validation rules) other scenarios where a field could return more than one kind of type.

If anyone decides to take ownership of this proposal, I think the bar will be set high to prove value and early attention should be placed on how it would affect code generation systems, especially ones based in Java/C/C++/ObjC.

How about the case when DB field supports string or number and requires API to adopt that fact? I mean when there is no way to change DB schema.

It seems possible to create my own ScalarType which would validate either string or int, and I don't see any issues using such scalar types, why just not put such types to graphql?

derekdon commented 4 years ago

I am fairly new to Graphql and I think this is the issue I've just hit and could do with advice on the correct approach to follow.

I set up a number of KeyValue pair content types in Contentful (KeyValuePairBoolean, KeyValuePairShortText etc), and in my Page content type I created a "params" reference to many of these key value pairs. However on trying to write a query for these I'm told:

"Fields "value" conflict because they return conflicting types Boolean and String. Use different aliases on the fields to fetch both if this was intentional."

              params {
                ... on ContentfulKeyValuePairBoolean {
                  value
                  key
                }
                ... on ContentfulKeyValuePairShortText {
                  value
                  key
                }
              }

Doing that of course does get rid of the error:

              params {
                ... on ContentfulKeyValuePairBoolean {
                  booleanValue: value
                  key
                }
                ... on ContentfulKeyValuePairShortText {
                  stringValue: value
                  key
                }
              }

However if that's the solution I might aswel just change the name of the fields in Contentful to the above so I can avoid the aliases.

I'm using Gatsby and looked at little at it's createSchemaCustomization / createTypes apis to see if there was a way to make value string or boolean, and perhaps try to leverage __typename... but I didn't have much luck with that and don't want to invest anymore time in it until I know for sure it's the right way to go here. But even after reading the above comments, it's not 100% clear on what approach I should take. Any advice would be much appricated. Probably worth mentioning it's a new (Contentful, Graphql, Gatsby) project and I have full control over each layer so can configure it whatever way is required.

xaun commented 4 years ago

Like with any union - it has to be prepared for any item in the union. I don't see that this poses any special problems.

Totally agree with this. I'm currently porting an existing data structure to graphQL which in TypeScript is this:

interface Question {
  options: string[] | { order: number, value: string }[]
}

I've just made a custom scalar type to validate this, not ideal:

// resolvers.ts
const validateQuestionOptions = (value: any) => {
  if (
    !(
      value.every((v: any) => typeof v === 'string') ||
      value.every(
        (v: any) => Number.isInteger(v.order) && typeof v.value === 'string'
      )
    )
  ) {
    throw new Error(
      'A question options: [String!] | [{ order: Int, value: String }!] '
    )
  }

  return value
}

export default {
  Query,
  // Mutation: {},
  // 
  QuestionOptions: new GraphQLScalarType({
    name: 'QuestionOptions',
    description:
      'Question options: [String!] | [{ order: Int, value: String }!] ',
    serialize: validateQuestionOptions,
    parseValue: validateQuestionOptions,
  }),
}

Not even sure if the above is legit as I'm new to GraphQL.. any suggestions on improving this would be great! Thanks

khawarizmus commented 4 years ago

Any progress on this issue?

I have a real-world use case where a mongoDB field can either be a referenceID or the actual document itself if we populate the field.

in that specific use case i would like to have a union type that can either be on object type or a simple string.

some sample code (typescript):

@modelOptions({ schemaOptions: { timestamps: true } })
@ObjectType()
export class Account {
  @Field()
  readonly _id: mongoose.Schema.Types.ObjectId;

  @Field()
  readonly createdAt: Date;

  @Field()
  readonly updatedAt: Date;

  @Field(() => EmailRefUnion)
  @prop({
    ref: 'Email',
    required: true,
    refType: mongoose.Schema.Types.ObjectId,
  })
  readonly _email: Ref<Email>;
  // Refers to Email

  @Field()
  @prop({ required: true })
  readonly password: string;

}

in this simple class which defines MongoDB schema and the objectType for Graphql at the same time using decorators i would like to have the possibility to define the EmailRefUnion as either an Email class or a String

import { createUnionType } from 'type-graphql';
import { Email } from '../../entities/email.schema';

export const EmailRefUnion = createUnionType({
  name: 'EmailRef', // the name of the GraphQL union
  types: () => [Email, String], // function that returns array of object types classes
});

But that is not supported in the spec yet

benjie commented 4 years ago

I'm currently trying to raise the priority of supporting scalar types in input unions in this comment: https://github.com/graphql/graphql-spec/pull/677/files/a35013f2dfb516c62f1955cc5a361d13a8181d8c#r370726872 It's currently :3rd_place_medal: (low) and I'm trying to raise it to :2nd_place_medal: (medium). If you agree, we'd love to hear why you think it's important - feel free to comment in that thread.

My proposed solution, the @oneOf directive (spec edits) could potentially be extended to output types.