mercurius-js / validation

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

Array or null validation error #57

Closed ziimakc closed 1 month ago

ziimakc commented 2 years ago

UPD. Found a workaround by setting directiveValidation: false, in my case it doesn't matter as I don't use directive mercurius validation.

For gql type CreateInput args nums and description are optional, so we can pass { nums: null, description: null } in CreateInput:

input CreateInput {
    description: String
    nums: [Int!]
}

Validation:

  CreateInput : {
    nums: {
      type: 'array',
      nullable: true,
      maxItems: 10,
      uniqueItems: true,
      items: { type: 'integer', minimum: 0, maximum: 9 },
    },
   description: { type: 'string', maxLength: 1000 }
  },

For some reason I get error when { nums: null } is passed as argument while { description: null } or { nums: [] } works:

      "message": "Failed Validation on arguments for field 'Mutation.smthCreate'",
      "stack":
          ValidationError: Failed Validation on arguments for field 'Mutation.smthCreate'
              at node_modules\.pnpm\mercurius-validation@3.0.0\node_modules\mercurius-validation\lib\validators\validator.js:32:15
              at executeField (node_modules\.pnpm\graphql@16.6.0\node_modules\graphql\execution\execute.js:481:20)
              at node_modules\.pnpm\graphql@16.6.0\node_modules\graphql\execution\execute.js:377:22
              at promiseReduce (node_modules\.pnpm\graphql@16.6.0\node_modules\graphql\jsutils\promiseReduce.js:23:9)
              at executeFieldsSerially (node_modules\.pnpm\graphql@16.6.0\node_modules\graphql\execution\execute.js:373:43)
              at executeOperation (node_modules\.pnpm\graphql@16.6.0\node_modules\graphql\execution\execute.js:347:14)
              at execute (node_modules\.pnpm\graphql@16.6.0\node_modules\graphql\execution\execute.js:136:20)
              at Object.fastifyGraphQl [as graphql] (node_modules\.pnpm\mercurius@10.1.1_graphql@16.6.0\node_modules\mercurius\index.js:654:29)
              at _Reply.graphql (node_modules\.pnpm\mercurius@10.1.1_graphql@16.6.0\node_modules\mercurius\index.js:340:16)
              at executeQuery (node_modules\.pnpm\mercurius@10.1.1_graphql@16.6.0\node_modules\mercurius\lib\routes.js:224:18)
      "path": [
        "smthCreate"
      ],
      "locations": [
        {
          "line": 3,
          "column": 3
        }
      ],
      "extensions": {
        "name": "ValidationError",
        "details": [
          {
            "instancePath": "/input/nums",
            "schemaPath": "https://mercurius.dev/validation/CreateInput/properties/nums/type",
            "keyword": "type",
            "params": {
              "type": "array"
            },
            "message": "must be array",
            "schema": "array",
            "parentSchema": {
              "type": "array",
              "maxItems": 10,
              "uniqueItems": true,
              "items": {
                "type": "integer",
                "minimum": 0,
                "maximum": 9
              },
              "$id": "https://mercurius.dev/validation/CreateInput/nums"
            },
            "data": null
          }
        ]
      }
    }
jonnydgreen commented 2 years ago

Hi, thanks for reporting (and for the workaround)! Would you be interested in submitting a PR for this?

ziimakc commented 2 years ago

No.

TimShilov commented 9 months ago

Just encountered this issue with the nullable Object and the directiveValidation: false workaround isn't working for some reason (and is not viable for me anyway as I use directives quite extensively).

I can see that PR is waiting for a review for over 6 months already. I have tested the #67 locally and it seems to be working just fine, I have not noticed any issues. Is there any reason why the PR isn't merged yet? Any help needed?

jonnydgreen commented 9 months ago

Hey @TimShilov , thank you for your offer of help! This PR dropped off my radar, apologies for this! The only thing blocking the merging is the resolution of one of the open thread which I think is okay to resolve, but if you have any ideas, I'm all ears! I'll aim to get this PR merged once everyone is happy :)