Lighthouse: How to handle required arguments: NonNull (!) vs @rules(apply: ["required"])

[robsontenorio]

Country ID is mandatory.

input UpdatePersonInput {
     country_id: Int!
     name: String
}

This is pretty clear, by graphql spec itself, and guide users about this API usage. API auto-docs will say it.

The above stuff is a pure schema definition. if user try to send a empty country_id an GraphQL Error is thrown (useless when handling errors on frontend, because there is no definition about fields keys; a big message is thrown).

When using @rules directive errors are friendly, because there are properly field keys. So i can handle it on frontend.

input UpdatePersonInput {
     country_id: Int @rules(apply: ["required"])
     name: String
}

However, it "breaks" the schema definition, because "officially" that field is not mandatory anymore. So API auto-docs get inconsistent (Eg, graphql-playground).

So... what is the best practice in this case?

[spawnia]

Consider the following schema

type User {
    last_name(
        arg: Boolean @rules(apply: ["required"])
        arg2: Boolean @rules(apply: ["required"])
    ): String
    full_name(
        arg: Boolean!
        arg2: Boolean!
    ): String
}

And a query like

{
    user {
        last_name
        full_name
    }
}

Will return a result like this

array:2 [
  "errors" => array:2 [
    0 => array:5 [
      "extensions" => array:1 [
        "validation" => array:2 [
          "arg" => array:1 [
            0 => "The arg field is required."
          ]
          "arg2" => array:1 [
            0 => "The arg2 field is required."
          ]
        ]
      ]
      "message" => "Validation failed for the field [user.last_name]"
      "category" => "validation"
      "locations" => array:1 [
        0 => array:2 [
          "line" => 5
          "column" => 17
        ]
      ]
      "path" => array:2 [
        0 => "user"
        1 => "last_name"
      ]
    ]
    1 => array:4 [
      "message" => "Argument "arg" of required type "Boolean!" was not provided."
      "category" => "graphql"
      "locations" => array:1 [
        0 => array:2 [
          "line" => 6
          "column" => 17
        ]
      ]
      "path" => array:2 [
        0 => "user"
        1 => "full_name"
      ]
    ]
  ]
  "data" => array:1 [
    "user" => array:3 [
      "last_name" => null
      "full_name" => null
    ]
  ]
]

As you noticed, the native validation is not as useful.

• The name of the argument is only inlined into the message, so you would have to do some string magic on that
• The second validation error is not even shown in native validation, so a user would have to try multiple times to get all validation errors

Because of this, i recommend using @rules for all your validation needs. However, i do get the point that it is not obvious from the schema, and it is more tedious to write out. I am open to ideas on how to improve on this.

[mfn]

Would it make sense, when using this kind of syntax to have a "simple" way to convert a graphql spec error into a library one, which has better reporting?

full_name(
        arg: Boolean! @rules
        arg2: Boolean!
    ): String

[spawnia]

@mfn that does not work in this case unfortunately, as webonyx stops producing errors after the first argument was faulty.

Working on a PR/issue for it at the moment.

[spawnia]

Opened an issue over at https://github.com/webonyx/graphql-php/issues/356

Feel free to comment on there.

[robsontenorio]

@spawnia

I have a simple suggestion to make it work.

In this case, when using only “!” ....

Input UserInput{
      name: String!
}

... implicitly apply @rules(apply: [“required”]

In this another case, just implicitly merge “required rule” with all another rules.

Input UserInput{
      name: String! @rules(apply: [“min:4”]
}

So, error message would be useful and docs will be consistent!

What do you think?

[robsontenorio]

Just updated the comment above. Sorry about formation.

[spawnia]

@robsontenorio

I like the idea behind this, although it is not easy to implement.

We do some magic on the schema currently, for example with pagination where we autogenerate the inbetween types. But this one is differently, as it requires to use a different schema for introspection then for execution. This is because you want the ! to show up in the schema explorer, but it has to be stripped when executing, otherwise we do not get to the point where our own validation is run.

We could work around this by deactivating the default query validation and replacing it with our own. This comes with its own set of problems though, as we will have to ensure that Scalars are still handled correctly.

For the moment, let's await feedback on https://github.com/webonyx/graphql-php/issues/356 before we move forward. I would rather not implement a workaround if we can avoid it.

[robsontenorio]

@spawnia @chrissm79 As an expert, do you think it is feasible?

According to Lighthouse's philosophy "write less do more", this would be pretty clear and GraphQL compliant/friendly, eg. using auto-docs/introspection:

Input UserInput{
      name: String!
}

Instead of

Input UserInput{
      name: String @rules(apply: ["required"])
}

It seems we won't have webonyx's support for now :(
https://github.com/webonyx/graphql-php/issues/356

[spawnia]

@robsontenorio The problem with this is that it would require to use a different schema for introspection than for execution.

We might work around this by just disabling the validation rule that webonyx uses for required arguments. That would require us to deliver a foolproof validation to ensure we do not break the API contract of the GraphQL schema.

[robsontenorio]

@spawnia Can you give us some directions how can it be handled? I'm gonna give a try, but i'm not used about Lighthouse source code, yet.

I really think it is a useful feature to keep schemas even more cleaner and docs compliant. I am very annoyed abou this :)

Input UserInput{
      name: String @rules(apply: ["required"])
}

instead of

Input UserInput{
      name: String !
}

[ElvenSpellmaker]

@spawnia FWIW I just stumbled across something similar to this (different from the OP's original issue but related) and it's kind of really undocumented.

I was trying to make a password field and had:

password: String! @rules(apply: ["min:8", "max:100"], messages: { max: "Passwords must be between 8 and 100 characters long."}) @hash,

And it wasn't doing the validation because while it's required by GraphQL it's not by Laravel.

I really think that by making it required by GraphQL it should add an implicit @rules(apply: ["required"]), or there should be ample warning that this isn't added in the documentation as I spent a considerable amount of time trying to discover that haha.

[ElvenSpellmaker]

@spawnia Actually the above behaviour is a bug I think.

If I pass password: "" it doesn't seem to apply rules in Laravel, but because it's been supplied it satisfies the ! on String!.

This means if someone passes an empty string, _all rules are ignored_, which clearly isn't right?

EDIT: With the current behaviour I can't get both create and update working as intended as if I put required for the update it'll force people to add fields I don't want them to add, but if they _do_ add them validation _must run_.

EDIT2: I think using "sometimes", "required", I can emulate what I want, that's quite confusing haha.

[spawnia]

I recommend to use different inputs for create and update.

[ElvenSpellmaker]

@spawnia I'm not using inputs at all (I don't get the point of them??), but I am using different arguments either way, in update they're not required, in create they are.

[spawnia]

You are free to make your own decisions.

input types are reusable and improve ergonomics when working with variables. Having distinct argument allows differentiating between required and non-required arguments.

[spawnia]

Good discussion was had, closing this issue. If something actionable comes up, we can improve validation.