Comments (31)
Example: Input validation and user-friendly error messages in GraphQL mutations on Medium.com
from graphql-js.
Thanks for the note, I'll add a comment in code to explain what's going on here.
+value
calls ToNumber internally.
num === num
is a cheap way to identify if num
is the value NaN
. NaN
is not equal to anything, including itself. Everything else will be a number.
from graphql-js.
FYI, if you follow this pattern, be sure to do validation for value coercion in addition to literal coercion.
I think this is an area we will need to cover in deeper documentation. As long as the validation has semantic value, I think this pattern is a good one. For example, at Facebook we have a type called "URL" which explains very clearly what kind of value will come from such a field and allows us to do validation if it's provided as input.
IMHO, doing this for "Email" makes a lot of sense, and I imagine you would want to use this not just to describe semantic input but also output
from graphql-js.
From slack talk:
currently we have “coerceLiteral” for this type of thing. However this is quite underspecified (especially in terms of errors) and we should firm this up before we finalize the spec
from graphql-js.
Ok, thank you.
So it could be useful to expose Kind
from lib/language
...
And also in the comment, it is only specified coerce
and not coerceLiteral
but if you don't provide both you get an error.
from graphql-js.
Nice catch. I'm leaving this open as a reminder to improve comments and docs
from graphql-js.
Could you give a quick explanations about the difference between both coerce
and coerceLitteral
?
The one that validate from the AST make sense but the other one, I am not sure when it is used...
from graphql-js.
There are two scenarios:
-
when used as the type of a field of an object, the value returned by the resolve function will be passed through coerce first. This ensures, for example, that Boolean is always Boolean, Int is always an integer, and String is always a string.
-
when input is provided as a query variable, the value of that query variable passed at runtime is first passed through coerce to ensure its of the correct type.
from graphql-js.
Allright, clear as crystal.
from graphql-js.
Not really related, but since I am digging into scalars....
This line for GraphQLFloat
seems wired
// [...]
coerce(value) {
var num = +value;
return num === num ? num : null; // <=
},
from graphql-js.
Sorry, if it sounds like a stupid question but the schema is supposed to be server-side only, or it can be embedded in client?
I ask because if I want to share some enhanced types (ex: GraphQLExtrasString({min:2, max:10}), it is important to know if the build size matters.
UPDATE: I am not just worried about the the build size, but also about features like Buffer
which is only available on the server.
from graphql-js.
It's expected that schema is represented client-side, usually in the form of code generation. You could imagine generating an NSObject, Java Object, or Flow type for every type represented in a GraphQL schema so clients can benefit from strong typed data.
Of course, if your server presents custom scalars, this can be a challenge for these tools. You could imagine defining a DateTime
scalar in GraphQL and wanting to ensure that they become JS Date and Java DateTime, etc. That's custom logic in those tools.
from graphql-js.
So to be clear, this is wrong?
/**
* Import dependencies
*/
import SomeLibWorkingOnlyServerSide from 'some-lib';
import {GraphQLScalarType} from 'graphql';
import {Kind} from 'graphql/lib/language';
import {GraphQLError} from 'graphql/lib/error';
/**
* Export enhanced String scalar type
*/
export default class String {
constructor(options = {}) {
return new GraphQLScalarType({
name: 'String',
coerce(originalValue) {
const {error, value} = SomeLibWorkingOnlyServerSide(originalValue, options);
if (error) {
throw new GraphQLError(error.message);
}
return value;
},
coerceLiteral(node) {
if (node.kind !== Kind.STRING) {
return null;
}
const {error, value} = SomeLibWorkingOnlyServerSide(node.value, options);
if (error) {
throw new GraphQLError(error.message, [node]);
}
return value;
}
});
}
}
from graphql-js.
Well, at a high level no, but this implementation in particular has a lot of issues:
To be clear: there's nothing wrong with defining custom scalars, that's exactly why the GraphQLScalarType
class constructor is exposed.
With this particular code there are a couple problems:
First, just from a JavaScript point of view, a class constructor should not return an object of another type (in fact, I'm surprised Babel isn't choking on this).
You should instead do:
export var CustomString = new GraphQLScalarType({
Next, you can't name your custom scalar String
since the GraphQL spec defines the behavior of the String
type which this doesn't adhere to, and this will result in multiple definitions of String
which will soon be a validation error (when the related issue is completed).
Here would be code that is perfectly fine:
/**
* Import dependencies
*/
import SomeLibWorkingOnlyServerSide from 'some-lib';
import {GraphQLScalarType} from 'graphql';
import {Kind} from 'graphql/lib/language';
import {GraphQLError} from 'graphql/lib/error';
/**
* Export enhanced String scalar type
*/
export default function CustomString(typeName, options = {}) {
return new GraphQLScalarType({
name: typeName,
coerce(originalValue) {
const {error, value} = SomeLibWorkingOnlyServerSide(originalValue, options);
if (error) {
throw new GraphQLError(error.message);
}
return value;
},
coerceLiteral(node) {
if (node.kind !== Kind.STRING) {
return null;
}
const {error, value} = SomeLibWorkingOnlyServerSide(value, options);
if (error) {
throw new GraphQLError(error.message, [node]);
}
return value;
}
});
}
from graphql-js.
a class constructor should not return an object of another type
Indeed.
So, do you think it would be useful if I create a package like graphql-extras
with some enhanced scalar types following this approach?
At first, my plan is to depends on Joi. And later make it more deps-free if people are enthusiastic.
Note: Joi doesn't work on Browser.
from graphql-js.
And thank you for your time/answers. I really appreciate it.
from graphql-js.
This seems fine - there's nothing wrong with custom scalars, the caveat is that client-side tools may not know about them and without building the rules for custom scalars into those tools, won't be as intelligent and helpful as possible.
from graphql-js.
Re: graphql-extras
, I would suggest a more descriptive name. Maybe graphql-custom-strings
or something like that.
from graphql-js.
graphql-validator-types
or graphql-validator-scalar
maybe....
from graphql-js.
Second thought, having custom
in the name could be more explicit. Easier to see that they are not built-in types. So I think it will be graphql-custom-scalars
.
Last question, about naming convention, some type ends with Type
(ex: GraphQLScalarType), some without (ex: GraphQLString). What is the rule?
from graphql-js.
I think there is a bug with the parser, a scalar created within a function is not recognised as a scalar.
const emailType = createScalar('Email') // return new GraphQLScalarType(...)
[...] // in Query
args: {
email: {
name: 'email',
type: new GraphQLNonNull(emailType)
}
},
I get this error
Argument email expected type Email! but got: "[email protected]".
And the coerce/coerceLiteral
of my custom scalar are not called.
Same when I define the type on a field
const userType = new GraphQLObjectType({
name: 'User',
fields: () => ({
email: {
type: emailType
}
})
});
I get this error
Field "email" of type Email must have a sub selection.
However it works if I use the same custom scalar directly (not created and returned by a function).
from graphql-js.
It's hard to see what your bug is without seeing how createScalar is implemented. It's almost certainly an issue within there.
Can you post the full code you're using to reproduce this issue on jsbin or requirebin?
from graphql-js.
Yes sure, I do it now.
from graphql-js.
While writing a clean example to reproduce the bug, it seems to works.
Must be something wrong in my implementation indeed, sorry about that.
from graphql-js.
If you figure out what went wrong, let me know. I'd like to make the error messages as descriptive as possible.
from graphql-js.
I have two questions related to custom scalar types:
- In the coerce method we don't have access to the AST, should one still use a GraphQLError or should a simple Error be used? It seems like the response's
errors[].locations
is set to[undefined]
in both cases, is this expected? http://facebook.github.io/graphql/#sec-Errors indicates that nolocations
should be set. - Like @SimonDegraeve points out
Kind
fromlib/language
is not exported (or is it perhaps available some other way?). Does that mean that one should use strings such as "StringValue" when checking the AST in thecoerceLiteral
method of a custom GraphQL type?
from graphql-js.
In the coerce method we don't have access to the AST, should one still use a GraphQLError or should a simple Error be used?
Right now we expect invalid coercions to result in returning null
, but throwing is reasonable. Would you mind opening a specific issue for this concern?
Like @SimonDegraeve points out Kind from lib/language is not exported (or is it perhaps available some other way?). Does that mean that one should use strings such as "StringValue" when checking the AST in the coerceLiteral method of a custom GraphQL type?
This has since changed. It's not exported from the main module, but it is exported from the sub-module:
import { Kind } from 'graphql/language';
However checking the string value is also acceptable if you prefer that
from graphql-js.
@leebyron thank you for the update about the submodule.
from graphql-js.
If anyone wants to know more on validation and custom scalars go here https://github.com/mugli/learning-graphql to custom types
from graphql-js.
@pyros2097 A big thank you
for the link. I managed to create a package here https://github.com/xpepermint/graphql-type-factory.
from graphql-js.
Thanks @ lot guys !
from graphql-js.
Related Issues (20)
- Collection of libraries and how they import from `graphql` HOT 20
- `process.env`, `globalThis`, and `typeof process` HOT 22
- Introspection queries don't support `@oneOf` HOT 2
- Tutorial data HOT 2
- astFromValue fails with a custom scalar serializing to an object value HOT 5
- In a response to a query about an Issue, the URL and other info is missing for links created with Reference editor button HOT 5
- author/committer -> user fields returning NULL for commits committed by user
- [email protected]
- npm link with graphql package breaks application HOT 2
- IHeyReally.com HOT 1
- Can I ask what is the progress here? Is there a solution being worked on? Do we have some timeline? Or progress with issue? Thanks!
- IHeyReally.com
- Typescript error with 16.9.0 (re ThunkObjMap) HOT 3
- Notice: default branch is now `16.x.x` HOT 2
- <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/12261526/38307428/4135/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-not-signed.svg" alt="CLA Not Signed" align="left" height="28" width="328"></a><br/><br /><ul><li><a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/12261526/38307428/4135/#/?version=2' target='_blank'>:x:</a> - login: @Heyitsquoracom / name: [email protected] . The commit (b6081f914f0c5c22ee48d26aff4b473bf17627ce) is not authorized under a signed CLA. <a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/12261526/38307428/4135/#/?version=2' target='_blank'>Please click here to be authorized</a>. For further assistance with EasyCLA, <a href='https://jira.linuxfoundation.org/servicedesk/customer/portal/4' target='_blank'>please submit a support request ticket</a>.</li></ul><!-- Date Modified: 2024-06-28 04:40:51.630768 -->
- Error with version v16.9.0: `Can't resolve '@apollo/subgraph/dist/directives''` HOT 1
- Type is incorrect in graphql ExecutionResult with new type GraphQLFormattedError HOT 1
- [email protected]
- Allow to call `resolveType` outside of `execute` context HOT 1
- Add Query to Fetch All Messages HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from graphql-js.