-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add "Any" scalar type #325
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this proposal is exciting. We've talked about this a few times, but getting the details right is important.
I've left some comments here to help clean up the spec language and catch some ambiguities, however next steps should be to link a reference implementation PR, and to hold discussion.
Thoughts from others? @robzhu @dschafer @stubailo @OlegIlyenko
spec/Section 3 -- Type System.md
Outdated
exact type of possible data is not known in advance or multiple types should be | ||
supported. | ||
|
||
Note: {null} value supplied as an argument or field value should be interpreted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"as an input value"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note feels out of place, as this statement is true for all scalar types, not just this new Any type. However I do think clarification is needed about how Any interacts with Non-Null.
spec/Section 3 -- Type System.md
Outdated
#### Any | ||
|
||
The Any scalar type represents any value that can be represented by underlying | ||
serialization protocol (including lists and maps). It is intended to be an opt-out type in case when the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also please correct the grammar issues with this sentence
spec/Section 3 -- Type System.md
Outdated
|
||
The Any scalar type represents any value that can be represented by underlying | ||
serialization protocol (including lists and maps). It is intended to be an opt-out type in case when the | ||
exact type of possible data is not known in advance or multiple types should be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think suggesting supporting multiple types with Any
isn't quite correct. Maybe a better way to phrase this is in terms of input types - where an input wishes to accept any kind of data.
as literal {null} and handled according to GraphQL rules for arguments and | ||
field values. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove extra line
spec/Section 3 -- Type System.md
Outdated
|
||
GraphQL servers should coerce raw value to the value that can be represented by | ||
underlying serialization protocol when possible otherwise they must raise a | ||
field error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence is confusing, I think there may be grammatical mistakes? Under what condition should a field error be raised?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leebyron Idea was to specify that error should be raised in case if returned type can't be represented in serialization format, e.g. NaN
in JS
spec/Section 7 -- Response.md
Outdated
@@ -58,6 +58,7 @@ the following JSON concepts: | |||
| Int | Number | | |||
| Float | Number | | |||
| Enum Value | String | | |||
| Any | Arbitrary JSON | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Any Value
is appropriate for the right hand side. We should avoid confusion that Any
is to be string-serialized JSON, which this could be misinterpreted as.
I also think that this proposal needs some strong support from real product teams (perhaps someone from AirBnB or Github?) rather than just citing graph.cool/scaffold/Apollo since those are generalized tools instead of product APIs. My primary concern (which I share with @dschafer) is that despite GraphQL being pervasive at Facebook there hasn't been a need for an |
One interesting question that comes to mind for me is whether
It would feel a bit odd to do that as it stands because But yeah, my primary concern is what @leebyron articulated: we really haven't seen a demand for this at Facebook. So when I think about potential use cases of this, there aren't a lot of positive examples that spring to mind... but the negative examples of someone lazily typing a field as I think there's a potential argument that the added friction that exists today to enable this functionality with a custom scalar is actively desirable. If this functionality were available as That being said, the absence of evidence at Facebook isn't evidence of absence in general, so definitely interested in hearing more about the use cases! |
I am not sure about this change: I can attest that people ask and want something like that when they first learn GraphQL ("I just want to have JSON/Map"), but I always argue that this is against the fundamental idea of GraphQL and it is not really supported. If it is really needed a custom Scalar can be created. I agree with @leebyron that I would like to see some real life examples. And while Programming Languages need something like that I am not sure about GraphQL: it is about describing an API and therefore fundamentally different from a PL. And as pointed out from @dschafer it adds complexity to the type system itself: Is |
I don't have an opinion on this - I think the current approach with a JSON scalar is okay, but it is something that comes up somewhat often so it might be nice to have a standard. In Optics, we use this to return data from our payment system which allows arbitrary tags and data, and we don't want to put all of the options in the schema. |
@leebyron I updated PR although it may still have some ambiguities as I'm not a native speaker.
I will start working on it.
@dschafer my idea is to treat it as
For that, you don't need
In JS it's a matter of two lines: For Sangria you can use snippet discouraged but provided by it's author. And probably similar packages exist for many popular GraphQL implementations. Also, support for We use IMHO, using |
Just my 5 cents. I would agree with others. I do see demand for JSON/Map but I'm yet to see a good compelling use-case for it (if you look at sangria's snippet, you will see a big disclaimer. :) ). For the most part, I heard about it in a context of the transition from untyped API. So I would rather prefer to avoid introduction of this feature unless we have a very compelling real-world use-case. |
@stubailo, can you please go into more detail? What are some examples for tags/data? Do they take on any standard shape? Why aren't they useful to include in the schema? How are you parsing this data and what assurances do you have about the shape of the deserialized object? |
I'll take a stab at adding some real product team input 😄 In the context of adopting GraphQL at XING SE and transitioning an untyped API to it (I think this is what @OlegIlyenko was referring to), we're also using an Our current use-case is two-fold: 1. Ease the transition of untyped APIs to GraphQL.For example we have a backend specific JSON error response that we did not yet model in a typed way but would like to already gradually adopt a typed API and pass this yet unmodelled part through at a certain node. 2. Provide custom directives on the schema to support developmentAs the different product teams integrating via GraphQL do not all use the same programming language, we try to make it possible to work directly with the schema definition as IDL. Similar to what @IvanGoncharov was describing we also define directives that need to receive arbitrary input. These include:
HTH, |
I might just be a GraphQL n00b, but my use case for this scalar is a dynamic form. My client needs to be able to ask my server "give me all the values for form X for user Y" (where "form X" could have any number of fields). I could represent that in totally uncontroversial structured GraphQL by storing each value as an object in an array:
or I could just have a JSON field with the value:
The latter is less bandwidth and seems easier to understand to me. |
I am trying to do this as well. I have two use cases:
I can see no way of implementing these endpoint without completely bypassing the graphql type system either via a JSON type or stringifying the data and passing another 'type' field which feels even worse. Given the difficulty of implementing these endpoints with GraphQL you might ask why I'm even trying to do so. The reason there is that there are many other APIs that can be easily typed with GraphQL and I'm trying to move our stack the direction of GraphQL + Relay Modern / Apollo and if not all of the endpoints can be expressed with GraphQL that means that I have to expose both a REST server + GraphQL server to the UI as opposed to exposing only GraphQL but backing it with the REST + Swagger server. Furthermore, it also means that I cannot migrate endpoints from REST to GraphQL seamlessly because the UI has to know about which endpoints are coming from REST and which are coming from GraphQL. P.S. I tried to implement this instead using a Union which includes Scalar types, but that isnt supported either. In swagger, you can specify that an api returns anyOf: typelist which isnt currently possible to express in graphql if typelist includes a scalar. That might be a way to allow an any type to exist in the system for people who want it (or not have it if you dont). This is further useful for apis which return an object on success or a plain string on error. |
I would prefer to implement the "any" concept by relaxing the restriction on unions and allowing a union of scalars and/or non-scalars as mentioned previously by @ofersadgat. Our primary user-case is the ability to create arrays of key-value pairs for property maps. I feel that the whole JSON scalar concept is an anti-pattern as it allows one to bypass the benefits of a strongly types API, instead of addressing the shortfalls directly. |
Why not adding this feature for development-only mode at least, with warnings etc? |
i also prefer allowing unions to have both objects and scalars (example: field "user" can be int-id, uuid or whole User object, depending on some settings/context/needs/permissions). This way is less prone to "make everything a void*" while still gives a lot of freedom, and allows for more cleaner APIs. (currently this can be done with 3 fields user_obj, user_id, user_uuid - but if its for each and every type around, that's alot of clutter) |
e5d241d
to
6c81ed8
Compare
In some of our tool, we need to be able to pass an arbitrary value as an argument of a directive, i.e. value which can be anything and can’t be described as a single type. The only workaround we found is to define
scalar JSON
which can accept any JSON value including objects and arrays.Moreover, this as popular approach in the community:
JSON
scalar in their schemascalar JSON
as an example in their documentation.AFAIK, this technique doesn’t contradict GraphQL spec and solves issues at hand.
At the moment, it has very limited support from tooling since it’s not a built-in type and introspection provides you no clue beyond the name of a scalar.
The idea behind this PR is to formalize existing practice for creating
scalar JSON
but in protocol agnostic way, that’s why I propose to useAny
as the name.The last argument I have is that all type system that I know have some form of opt-out type:
any
in Flow and TypScript,void*
in C/C++,sql_variant
in SQL andObject
class in C#/JAVA.