Skip to content

Clarify allowed Values for Scalar types #688

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

Open
fluidsonic opened this issue Feb 9, 2020 · 11 comments
Open

Clarify allowed Values for Scalar types #688

fluidsonic opened this issue Feb 9, 2020 · 11 comments
Labels
🤷‍♀️ Ambiguity An issue/PR which identifies or fixes spec ambiguity

Comments

@fluidsonic
Copy link
Contributor

fluidsonic commented Feb 9, 2020

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:

  • It always has exactly two Int! values: start end endInclusive.
  • The typical approach for a type with multiple properties would be to create a type IntRange with two Int! fields for output, and an input IntRangeInput with the same two fields for input.

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.

@fluidsonic fluidsonic changed the title Clarify allowed Values for scalar types Clarify allowed Values for Scalar types Feb 9, 2020
@andimarek
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor Author

@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
Copy link
Contributor

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

@conradreuter
Copy link

conradreuter commented Feb 26, 2020

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
Copy link
Contributor Author

@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
Copy link
Contributor

andimarek commented Feb 27, 2020

@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
Copy link
Contributor Author

fluidsonic commented Feb 27, 2020

@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
Copy link
Contributor

@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!

@IvanGoncharov IvanGoncharov added the 🤷‍♀️ Ambiguity An issue/PR which identifies or fixes spec ambiguity label Mar 2, 2020
@kdawgwilk
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤷‍♀️ Ambiguity An issue/PR which identifies or fixes spec ambiguity
Projects
None yet
Development

No branches or pull requests

6 participants