Uniforms: graphql schema does not respect required fields

[macrozone]

if you don't pass the second arg to new GraphQLBridge(schema, validator, extras), validation crashes, because it needs a function.

this is odd, because in the code GraphQLBridge tries to check whether a field is required or not base on the schema itself (defined with !) (it sets required: field.type instanceof graphql.GraphQLNonNull)

But even with an empty validation function () =>null, it does not work. GraphQLNonNull seems to not be used here.

I think, GraphQLBridge should have a built-in validator that checks for GraphQLNonNull

[radekmie]

Hi, @macrozone. It does make sense but only partially. I agree that there _might_ be added a default validator, which will check this constraint, but it's very hard actually to make one. For example: is '' (empty string) OK for a required String? Well, it depends on the case. Now, because of that, it's definitely incorrect to _add_ one (like an additional check, besides the user provided one). To make it clear, there's no validator at all.

I'll think about that more, as maybe using utilities/coerceValue will be enough for a basic validation, but it's maybe too much for a rarely used default.

[macrozone]

@radekmie yeah, agree on that. Graphql accepts empty strings in required fields. I am just unsure, will uniforms clean empty text-fields to "" or to null? (probably "")

I am just toying with the idea of having a simpel first form based on mutations on a backend (with 0-config) and taking as much information from the schema as possible.

[radekmie]

Nope, uniforms won't do any cleaning, so if the field called onChange with an empty string (for example it's a default value), it'll get an empty string and undefined otherwise.

I actually like the idea: are you doing it somewhere publicly?

[radekmie]

No answer so far, I'm closing. Feel free to comment anyway.

[simplecommerce]

Did anything come out of this issue?
From what I can see, no, but I also think it could be a good feature to have.
I couldn't figure out on my end how to implement it though.

[radekmie]

@simplecommerce: No, there's nothing happening in the core in this regard. As already said, it's out of the scope of uniforms packages. However, if you'd make one, feel free to let us know here - we'll gladly add it to the docs.

[simplecommerce]

@radekmie Thanks for your reply, I did something on my end in order to achieve it, but I doubt its the best idea or its optimized enough. If you want to take a look at it, I can share what I wrote in order to use it on my end.

The idea is I am using the available methods in the GraphQL bridge in order to loop through the fields of the schema, and I am validating every field, including sub fields, when they are required and are empty.

The problem I saw, was putting it in the validator didn't work as it would cause performance issues.
I had to use onValidate and setTimeout to prevent it from executing every time you change a value while there is an error.

My implementation probably isn't the best, so any expertise on how to make it efficient would probably be better if anyone is going to use it also.

[macrozone]

sorry for my silence, i forgot about that and did not use the GraphQLBridge since then :-/

[radekmie]

@simplecommerce: Post the code somewhere where we could see and test it, please. Our reproduction CodeSandbox would be ideal, but choose what suits you best. I'll try to get to it _soon_. And maybe somebody will be even faster.

[simplecommerce]

@radekmie ok I will try to make a test repo today

[simplecommerce]

@radekmie Ok so here is the test sandbox, it is not perfect but it gives you an idea of how I am implementing it.

https://codesandbox.io/s/validate-required-fields-5j5zp?file=/src/App.js

You guys can probably figure out a better and more optimized approach.

My validation code can be found in the GraphQLBridge.js file under the validateRequiredFields method.

Update

In my example, the validation doesn't seem to work with a non null Array type declared like this:

input SomeInput {
  name: String! 
  # this doesn't work
  items: [String]! 
}

After doing some tests, it seems to be inside the bridge itself, it doesn't seem to consider if an Array can be non-nullable also.
Taken from the specs here: https://graphql.org/learn/schema/#lists-and-non-null

I am not sure how to fix this part.

[radekmie]

I've just taken a quick look at your code and it looks correct. It could be, of course, simplified, but it's fine for a PoC. From my quick profiling session, it takes ~1ms for validation, which is completely fine for real-time validation.

Performance-wise, consider this change:

-const { required = false } = this.getProps(fieldPath);
+const required = isNonNullType(this.getField(fieldPath)).type;

And about the _update_ from your last comment: it works. By default, uniforms will populate it with an empty array. I've checked it manually, and filling the first 5 fields (as these are required) shows a comments: [] in the alert.

[simplecommerce]

@radekmie Ok I will test it again on my end to make sure, thanks for your feedback!