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

support for Scalar constraints akin to xml schema facets #184

Closed
JeSuisNikhil opened this issue Jun 11, 2016 · 7 comments
Closed

support for Scalar constraints akin to xml schema facets #184

JeSuisNikhil opened this issue Jun 11, 2016 · 7 comments
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)

Comments

@JeSuisNikhil
Copy link

It would be nice to define constraints like minValue, maxValue and regular expression based validation strings for scalars. Is there a plan for this?

A reference for some restrictions http://www.w3schools.com/xml/schema_facets.asp

@leebyron
Copy link
Collaborator

leebyron commented Jul 2, 2016

There is support for something similar to this - however it's not represented by the introspection system. Any custom Scalar has the ability to programmatically determine if a value provided is invalid. However since this is programmatic, it can't be serialized via the introspection system.

@JeSuisNikhil
Copy link
Author

it can't be serialized via the introspection system

Not sure what you mean by that. Are you saying that I could write a scalar definition for an extended String (say) and then add restriction like properties (such as maxLength, minLength) to it? And that this scalar would not be serialized with it's properties since it's programmatic?

I was hoping for these to be more at the field definition level rather than the custom scalar definition.
Something like this maybe?

type Human {
  id: {
    type: String
    restrictions:[{minLength: 0}]
  }
  name: {
    type: String
    restrictions:[{maxLength: 30}]
  }
  homePlanet: String
}

I don't know if this is breaking another JSON or GraphQL rule.

@leebyron
Copy link
Collaborator

leebyron commented Jul 2, 2016

Ah perhaps I misunderstood the original proposal. You're not suggesting constraints on the scalar types themselves, but suggesting constraints on the values which can be provided for fields.

Do you have a use case in mind? Given your example, what would happen if the name of a human was in fact 31 characters? Would you present an Error instead of returning it? What what a GraphQL client do with the information that name can only return a string of maxLength 30?

@JeSuisNikhil
Copy link
Author

Another clarification that the restriction would be on the input field. May not necessarily apply to the output field. Though I guess that could lead to some confusion.

One obvious example I can think of is age where minValue is 0. Other real world examples could be UUID, SSN, phone number, a regex based restriction for email address.

Yes, I would present an error if the input field value specified is not valid. And it would be at the same level as the type itself being invalid (providing a number in place of a string). For the output it would depend I think and maybe it should be configurable that the output restriction apply too.

A graphql client like a UI front end could query field metadata and read it into a ui model of sorts and then apply that UI validation on the UI component. So if the API says that the age should be greater than 0, the front end would show a validation error if the user entered a number less than 0.

There are a lot of implementation specifics I'm skipping here :)

@leebyron
Copy link
Collaborator

leebyron commented Jul 2, 2016

Ahhhh ok I'm understanding better - these would be for Input Object types. It's maybe better to discuss this as specific to inputs, I think there are a lot of tricky issues with limited value when attempting to apply constraints to output types.

Yes, so proposals similar to this have been brought up a few times before, mostly faded out due to added complexity, limited expressivity, and the existing ability to provide custom scalars and/or implement custom resolve to throw errors if inputs are bad, however I still think there's merit in continuing to discuss this idea.

A particularly interesting way to think about this is as, similar to input types, a way to document how a various input should work.

I think a possibly dangerous outcome is that these restrictions could multiply and result in a lot of added complexity in GraphQL with marginally less value. Schema Facets describes 12 different restrictions which seems like a lot to me.

I'm curious what the common uses of something like this would be in practice? For example if there are a very few very common uses then perhaps we should optimize for solving those first with the least complexity.

@JeSuisNikhil
Copy link
Author

As for common uses, I think there is a valid use case every time a database is fronted with a graphql api. If a name column in a table has a maxlength then so should the graphql input field. However, one could argue that this sort of validation can be provided in a business/dao layer between the graphql and the database. But I think there is some merit in providing basic input type value restrictions at the type definition.

As for limiting the restrictions, I can immediately reduce 12 to 6 namely maxVal, minVal, maxLen, minLen, precision and regex (assuming inclusive for max and min values). Since regex can cover maxLen and minLen I could bring it down to 4.
And now these restriction can be context limited by the type to which they apply. regex apply to strings, max and min apply to ints and precision to floats.

This is how I imagine a restricted type would look like then.

type Human {
  id: String![0-9a-z]{16} // 16 digit alphanumeric
  name: String![a-zA-Z ]{1,30} // 01-30 characters
  email: String+@+ // must have @
  age: int{1,} // min value 1, max val undefined
  money: float{2} // precision 2 decimal places
}

The restrictions would be optional of course.

@leebyron
Copy link
Collaborator

leebyron commented Oct 2, 2018

Rejecting this aging strawman since it did not develop further. This would require significant additions to the parser and introspection system and it's not clear there is a compelling motivation worth this complexity.

@leebyron leebyron added the 🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md) label Oct 2, 2018
@leebyron leebyron closed this as completed Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

2 participants