-
-
Notifications
You must be signed in to change notification settings - Fork 564
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
Spec Compliance: Strict input coercion for scalars #278
Comments
I am using laravel-graphql which uses an older version: While looking through the repo some more, i found pull request #171 which implements strict input type coercion. For a golden period of time, this actually was spec-compliant. Unfortunately, along comes #248 which reverts back treating input coercion for variables in I already opened an issue in |
Yeah, there were different opinions on this subject and the reference implementation choice had won this dispute in past. So I think the best way to fix it is to report this issue in the reference implementation repo. If they admit it then we can revert back to our original fix (or use the solution they introduce eventually). |
Actually, that is exactly what i did: graphql/graphql-js#1324 I really hope they will respond soon and take care of this. There is an argument to be made for doing some coercion for input variables: Since graphql does not specify a transport format, theoretically there could be a format that does not have native support for some of the scalars that graphql offers. Literals are different in that regard, since the query string itself has a well known format. Realistically, i have not heard of a case where variables are provided through anything else than a JSON object. Because of this, it is reasonable to assume that variables do have a direct equivalent to the scalar types defined by graphql. Should a use case pop up where variables are passed in a format that, for example, can only supply string values - then a more loose coercion would be necessary. Suppose the reference implementation stays with their current way of doing things, how would you proceed? I can understand not wanting to deviate from them, but on the other, deviating from what the spec is doing seems even worse. Maybe there is a way to both maintain feature parity with the reference implementation and offer spec-compliant strict input coercion for the 99% use case, which is variables passed through JSON, by offering a configuration option for it? |
@spawnia Reference implementation can't be non-spec-compliant %) So if they refuse to change, they will also have to clarify/change the spec (they are pretty much the same people who write the spec). One thing I can say for sure that we won't have any configuration option. Excessive configuration options are an easy-at-first path which has many side effects in the long run (and not just technical). |
A PR in the reference implementation is on the way to switch to strict input type coercion: graphql/graphql-js#1382 |
Good news. I regret that I've published 0.12 with a reverted behavior. Will probably have to fast-forward to 0.13 with a recommendation to avoid 0.12 if scalar serialization is important. |
This reverts commit 975c9fe
This reverts commit 975c9fe
This reverts commit 975c9fe
The GraphQL specification defines strict rules for input coercion, especially of scalar types: http://facebook.github.io/graphql/October2016/#sec-Scalars
However, inputs that are passed in as variables are not validated accordingly, while literals are.
parseValue
mostly behaves the same asserialize
- which allows for some loose coercion.If i understand the specification correctly, such loose coercion is only allowed for results - outbound information that the server sends to the client. It makes sense in this case, the server should not break the contract it defines in its schema.
Inputs, regardless if they are provided as literals or as variables, are inbound information which come from the client and are sent to the server. In that case, strict validation makes absolute sense - if the client sends bad data, the server should not quietly coerce the value but rather throw an error.
While literals and variables have to be handled differently at first, they both fall into the category of Input Coercion. The spec is really clear on how inputs should be handled, see Section 6.1.2:
What is currently happening is that the result coercion rules, which are different from input coercion rules, are applied to input variables. On the other hand, input literals are treated different than input variables - while according to the specification, they should be handled the same in regards to the coercion.
It seems like there is a fundamental misunderstanding here. If it is on my side, maybe you can clear it up for me? If not, i think resolving this issue to ensure spec compliance requires breaking changes to the implementation.
The text was updated successfully, but these errors were encountered: