Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Union of Non-Object Types #291

Closed
jamespedid opened this issue Feb 11, 2016 · 6 comments
Closed

Union of Non-Object Types #291

jamespedid opened this issue Feb 11, 2016 · 6 comments

Comments

@jamespedid
Copy link

I've run into a snag where I'm trying to create an input type that will have a where API that is similar to the one used by the sequelize library, where you can have where clauses like:

myStuff(where: {name: 'Jim', age: {$gt: 25}}) {
  socialSecurityNumber
  creditCardInfo
  motherMaidenName
}

Here, the key-value of the where clause is desired to be: name = or name = operand. So naturally I want to create a union type that allows for a scalar value or a name.

I might be able to work around with a simple object type, but this clutters the query and parsing:

myStuff(where: {name: {value: 'Jim', operator: 'eq'}, age: {value: 25, operator: '$gt'}}) {
  socialSecurityNumber
  creditCardInfo
  motherMaidenName
}

I don't see why a union of different scalar types is a bad thing, especially when I'm telling the resolveType function how to resolve the value's type.

@bringking
Copy link

This is slightly related to #290, I am trying to solve passing "where" objects as input types as well.. I ran into this issue as well in my attempts.

@leebyron
Copy link
Contributor

This is interesting as an initial use case, but maybe challenging to pull off.

The first part is identifying exactly what it means to be a union type. Input types are interesting in that the types are really only used for some static validation, but at runtime they're used as coarse input validation.

The existing UnionType is probably not the right tool here. The resolveType function you're referring to expects to receive internal values, like whatever ORM you might be using, and not input values which often start as raw JSON, but can also be GraphQL AST in the case where you're writing the input directly into your query as your examples do.

Also, you probably don't want to have to declare a named union types just for input like this, you'd likely rather define that inline.

One thought I had was just simply trying all validation functions in the order that you run them. So if you specify that an input will be String | Int then try to interpret it as a String first, and if that fails then try to interpret it an Int.

Another related proposal to this is intersection types. The ability to say that some input is SomeEnum & OtherEnum can be useful for variables that are used in multiple places. In these cases we would interpret it as all types specified and ensure that the interpretations are all equivalent in order to continue.

Finally, as I mentioned in #290, I think sometimes it's reasonable to just say that some input is just dynamically typed - and let GraphQL do the minimum input validation. That may be reasonable for some highly dynamic cases or where actually defining a whole input type system to match a bunch of essentially CRUD operations feels overly burdensome. The expectation with this is that your business logic would still be responsible for doing any specific validations if relevant.

Thoughts?

@jamespedid
Copy link
Author

I'm definitely in favor for "dynamic types" that limit the amount of input checking, for situations where the input type truly should be variable, or unknown at design time. Personally, I think that I should not only be able to resolve the type at runtime, but also the value, from the raw JSON string. A good example of this might to be to interpret a particular string that matches a date regex in one way as a DateTime object, otherwise I would want it to remain as a string, or some other situation like that.

@freiksenet
Copy link
Contributor

I am very wary of adding untyped stuff to GraphQL, because one of the great parts of GraphQL is that it actually validates things for you, including input objects in mutations. Another concern is that dynamic input objects will pave way for dynamic non-input objects :) I know I am invoking a slippery slope argument here, but I still think those are not a very good idea. :) At very least we will have a discrepancy between what is possible to pass as input objects and what one can return, because one will be typed and another won't.

Also note that for CRUD it's possible to generate input object, eg like I described here.

RE: unions - I think unions/intersections for input objects would be a very good idea. I think trying resolve functions in order is one way, other way is to have predicate (like resolveType), but for AST/raw values, that will return a corresponding type. It's already possible to implement a Scalar that will act like that, so this would just be a simplification of that logic.

@leebyron
Copy link
Contributor

Agreed, @freiksenet - I think subtle dynamicism is dangerous. Fully dynamicism may be appropriate at some points though, which I'm considering drafting a more concrete proposal for (similar to #172).

As @jamespedid points out, there are cases where you truly are dealing with dynamic data at runtime and you cannot know their types ahead of time, and there should eventually be a type that helps represent this (flow calls it "mixed" or "any")

@shtse8
Copy link

shtse8 commented Nov 27, 2017

Any updates on union scalar types? I have created a scalar type called ANY and do formating for workaround solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants