Skip to content
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

GraphQLScalars are not intended for email, password, etc. validation #168

Open
jedwards1211 opened this issue Jul 20, 2016 · 3 comments
Open

Comments

@jedwards1211
Copy link
Contributor

jedwards1211 commented Jul 20, 2016

See graphql/graphql-js#433 (comment)

Basically Lee says GraphQLScalars should only be used if the field is represented as a different underlying type, hence they shouldn't be used to validate emails and passwords, which are internally stored as strings.

@jedwards1211 jedwards1211 changed the title Using GraphQLScalars to represent emails and passwords violates best practice (according to Lee Byron) GraphQLScalars are not intended for email, password, etc. validation Jul 20, 2016
@leebyron
Copy link

leebyron commented Jul 21, 2016

Just following up here - this is not to say that if you do this that you're not doing GraphQL correctly or anything like that - the technology is young and experimentation with this technique is warranted and good. My opinion expressed on the other thread is that on balance the pros and cons have in the past weighed out in favor of doing validation elsewhere and keeping the type system simpler, but you may find other conclusions.

One concern is that if you ever foray into code generation based on a GraphQL schema for any client (for example, generating Java models for an Android app), then you'll need to teach those scripts what to do when they encounter an Email type, and every other custom scalar, where generalizing around a String type would simplify such an operation.

Of course, if you're not doing such code generation, then that wouldn't be a concern :)

@mattkrick
Copy link
Owner

@leebyron thanks for following up! Very interesting point & what you alluded to is the very thing I want to try. Similar to how an introspection query can create a client-safe schema, I'd like to build something that creates client-safe type validation. At a cursory glance, it looks like I'll just have to polyfill the GraphQLError, but I haven't had time to dig in fully. The end goal is an output that I can plugin directly into something like yup to achieve universal (albeit synchronous) validation.

@jedwards1211
Copy link
Contributor Author

Yeah I am curious to see how this turns out. You'll need to do validation in parseValue (and probably call that from parseLiteral) if you want the validation to take place when query parameters are used. One of the things I've been wondering is how easily the errors can be linked to specific fields to be displayed on the client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants