mercurius-js / validation

Adds configurable validation support to Mercurius.
MIT License
29 stars 6 forks source link

mercurius-validation doesn't support nullable type correctly #30

Closed paolochiodi closed 2 years ago

paolochiodi commented 2 years ago

According to GraphQL spec optional input values (that is, the one without exclamation mark) can receive a null value that is different than not providing any input (see http://spec.graphql.org/October2021/#sec-Null-Value).

Mercurius supports this behaviour and will correctly transform any null value input to null. mercurius-validation instead transforms the null value into an empty string. This is because:

The problem is mercurius-validation is inferring the wrong type and not taking nullable types into consideration.

The inferJSONSchemaType functionion (https://github.com/mercurius-js/validation/blob/main/lib/utils.js#L38-L49) should return different json schema types based on whether the original GraphQL type was nullable or not. I.e. { type: 'string' } for String! and { type: ['string', 'null'] } for String. This issue may be caused by getTypeInfo (https://github.com/mercurius-js/validation/blob/main/lib/utils.js#L51-L54) which removes the nullable information from the type.

Code to reproduce the issue: https://gist.github.com/paolochiodi/93a75b1b1cd551a745b6d8d77f83e1a4 Expected behaviour: no failure Current behaviour: returns error due to assertion failing. Removing Mercurius-validation by commenting out line https://gist.github.com/paolochiodi/93a75b1b1cd551a745b6d8d77f83e1a4#file-index-js-L29 will make the code run

jonnydgreen commented 2 years ago

Good spot! Would you like to submit a PR to fix this?

simoneb commented 2 years ago

I'm looking into this

mcollina commented 2 years ago

Thanks @simoneb