graphql / graphql-spec

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

Clarify allowed Values for Scalar types #688

Open fluidsonic opened 4 years ago

fluidsonic commented 4 years ago

The spec makes no statement about what GraphQL input values are valid for a custom Scalar value.


Let's take an IntRange type for example:

That's unnecessary for such a simple type, so I'd love to represent it as a Scalar instead. But because the type comprised of two values it cannot be represented by any other built-in Scalar type except String. The latter would require a custom serialization format like "start...endInclusive".

According to the spec Scalar represents primitive values.

The most basic type is a Scalar. A scalar represents a primitive value, like a string or an integer.

The spec doesn't really say what primitive means.


My idea is now to simply use a custom Scalar with an object representation for input and output.

Output it's simple because the spec basically allows any format:

GraphQL scalars are serialized according to the serialization format being used. There may be a most appropriate serialized primitive for each given scalar type, and the server should produce each primitive where appropriate.

So I can use a JSON object:

{ "start": 1, "endInclusive": 10 }

Input coercing for custom scalars isn't defined beyond the following:

If a GraphQL server expects a scalar type as input to an argument, coercion is observable and the rules must be well defined. If an input value does not match a coercion rule, a query error must be raised.

Basically the server can accept any value it wants to as long as it clearly defines that. That theoretically allows for using an ObjectValue to represent a scalar IntRange:

{ start: 1, endInclusive: 10 }

Conclusion

Something like IntRange could be seen as a primitive and thus be implemented as a Scalar. It could be represented using a JSON object for output and variable input, and as ObjectValue for input in a GraphQL document.

However in the spec it is not clear whether that is acceptable or not. Theoretically that approach can collide with the Input Object Field Names validation.

I suggest to clearly define what Values are valid to use for a Scalar.

andimarek commented 4 years ago

@fluidsonic it is definitely allowed and custom scalars for JSON like structures do exactly this: they accept an input object AST as input element.

But it could be more clear that arbitrary AST elements are accepted, yes. Have a look at this blog which fills in some details where the spec is not so clear (yet).

fluidsonic commented 4 years ago

Thanks @andimarek. Happy to hear that all are supported and that the spec (will) become clearer here in the future.

At least the rule Input Object Field Names should be improved earlier. If applied strictly as specified it wouldn't allow an object value for scalars:

  1. For each Input Object Field inputField in the document
  2. Let inputFieldName be the Name of inputField.
  3. Let inputFieldDefinition be the input field definition provided by the parent input object type named inputFieldName.
  4. inputFieldDefinition must exist.

In (4) inputFieldDefinition cannot exist for a scalar type as it doesn't have field definitions.

sungam3r commented 4 years ago

@fluidsonic The fact is that Input Object and Scalar are two different things.

See this - https://spec.graphql.org/draft/#IsInputType():

If type is a Scalar, Enum, or Input Object type

That is, IMO it is a mistake to extend the effect of the validation rule you quoted above to scalars. But these are my assumptions. I am starting to interpret the specification. Saying it explicitly in the specification itself would certainly be better.

fluidsonic commented 4 years ago

@sungam3r the rule name "Input Object Field Names" is misleading in the spec. The validation actually validates (Input) Object Value Field Names.

I'm not suggesting to extend the validation to Scalars. I'm saying that given the current wording it's impossible to use an Object Value for Scalars because according to that rule they're limited to Input Objects already.

The spec is also inconsistent with name naming between "Object Value" and "Input Object Value". They both seem to refer to the same thing.

sungam3r commented 4 years ago

I agree that something in the terminology is definitely worth changing.

conradreuter commented 4 years ago

The spec states that:

custom scalars [...] may be represented in whichever relevant format a given serialization format may support.

(Source: http://spec.graphql.org/draft/#sel-DAPJDLAAENBAAvtE)

I would say that's clear enough.

For JSON as a serialization format that translates to "Whatever can be represented in JSON".

fluidsonic commented 4 years ago

@conradreuter serialization only covers variables (input) and response serialization (output).

It does not state what kinds of GraphQL values are allowed as scalar values, nor does it solve the conflict with the input object value validation rule.

andimarek commented 4 years ago

@fluidsonic about your previous remark about the "Input Object Field Names" rule:

this rules applies only to Input Object types not to all Input Object Asts (Literal)

For example: for a schema:

scalar HelloWorldScalar
type Query {
  echo(arg: HelloWorldScalar) String
}

and a query:

{echo(arg: {hello: "world"}) }

This rule never applies because the AST {hello: "world"} is passed into the parseLiteral function of the custom scalar and the cited rule is not invoked.

To make it a bit more complex:

scalar HelloWorldScalar
input ComplexArg {
  customScalar: HelloWorldScalar
}
type Query {
  echo(arg: ComplexArg) String
}

and a query:

{echo(arg: {customScalar: {hello: "world"}}) }

Here the rule applies for {customScalar: {hello: "world"}} but only for the customScalar field: This field there is indeed a InputFieldDefinition customScalar: HelloWorldScalar. The value of it is {hello: "world"} which again doesn't apply to the rule because it is as a whole one value which is handled by the custom scalar.

I hope that helps.

fluidsonic commented 4 years ago

@andimarek I agree that the rule may be intended to mean just that. In the way it's written however that's not clear.

• Let inputFieldDefinition be the input field definition provided by the parent input object type named inputFieldName. • inputFieldDefinition must exist.

If I follow that rule as written, inputFieldDefinition wouldn't exist because it's a scalar field type. Hence inputFieldDefinition must exist fails and the rule fails. The rule is not limited specifically to Input Types.

andimarek commented 4 years ago

@fluidsonic you are right, this could be improved.

Have a look how it is implemented in GraphQL.js: https://github.com/graphql/graphql-js/blob/master/src/validation/rules/ValuesOfCorrectTypeRule.js#L47

Maybe you want to submit a clarification to the spec? That would be great!

kdawgwilk commented 3 years ago

I think it's also worth noting that some widely adopted GraphQL paradigms are making use of this ambiguity already e.g. Apollo Federation spec makes use of an _Any scalar type that is treated as an arbitrary object. There are already several implementations of this spec in various languages