express-joi-validation icon indicating copy to clipboard operation
express-joi-validation copied to clipboard

Can I use this with number()?

Open danielbrauer opened this issue 4 years ago • 15 comments

This validator looks like what I need, but as soon as I try to put a number in the schema I get horrible errors. Here is reproduction code, just slightly modified version of your type extraction example:

import * as Joi from '@hapi/joi'
import * as express from 'express'
import {
  ContainerTypes,
  ValidatedRequest,
  ValidatedRequestSchema,
  createValidator
} from 'express-joi-validation'

const app = express()
const validator = createValidator()

const querySchema = Joi.object({
  name: Joi.number().required()
})

interface HelloRequestSchema extends ValidatedRequestSchema {
  [ContainerTypes.Query]: {
    name: number
  }
}

app.get(
  '/hello',
  validator.query(querySchema),
  (req: ValidatedRequest<HelloRequestSchema>, res) => { // No overload matches this call
    res.end(`Hello ${req.query.name}!`)
  }
)

This produces the error:

No overload matches this call.
  Overload 1 of 4, '(path: PathParams, ...handlers: RequestHandler<any, any, any, ParsedQs>[]): Express', gave the following error.
    Argument of type '(req: ValidatedRequest<HelloRequestSchema>, res: Response<any>) => void' is not assignable to parameter of type 'RequestHandler<any, any, any, ParsedQs>'.
      Types of parameters 'req' and 'req' are incompatible.
        Type 'Request<any, any, any, ParsedQs>' is not assignable to type 'ValidatedRequest<HelloRequestSchema>'.
          Types of property 'query' are incompatible.
            Property 'name' is missing in type 'ParsedQs' but required in type '{ name: number; }'.
  Overload 2 of 4, '(path: PathParams, ...handlers: RequestHandlerParams<any, any, any, ParsedQs>[]): Express', gave the following error.
    Argument of type '(req: ValidatedRequest<HelloRequestSchema>, res: Response<any>) => void' is not assignable to parameter of type 'RequestHandlerParams<any, any, any, ParsedQs>'.
      Type '(req: ValidatedRequest<HelloRequestSchema>, res: Response<any>) => void' is not assignable to type 'RequestHandler<any, any, any, ParsedQs>'.
        Types of parameters 'req' and 'req' are incompatible.
          Type 'Request<any, any, any, ParsedQs>' is not assignable to type 'ValidatedRequest<HelloRequestSchema>'.ts(2769)

I am using:

"@hapi/joi": "15",
"@types/hapi__joi": "15",
"express-joi-validation": "^4.0.3",

Updated to remove type extraction: the issue happens even without it

danielbrauer avatar Jun 28 '20 14:06 danielbrauer

@danielbrauer could you you npm install [email protected] -S and see if that helps? Looks like a change to the express typings caused this and it's not related specifically to any Joi number etc. types

evanshortiss avatar Jul 15 '20 16:07 evanshortiss

@danielbrauer in case you're curious what the patch is - check this commit.

I tested locally using your example and by changing my node_modules/express-joi-validation/express-joi-validation.d.ts to have that change

evanshortiss avatar Jul 15 '20 16:07 evanshortiss

Thanks for addressing this! With the new version, I get exactly one compilation error:

node_modules/express-joi-validation/express-joi-validation.d.ts(28,42): error TS2314: Generic type 'ValidationResult<T>' requires 1 type argument(s).

danielbrauer avatar Jul 18 '20 20:07 danielbrauer

@danielbrauer can you update to Joi v16 and Joi types v16? AFAICT that'll resolve the new issue, i.e npm i @hapi/joi@16 @types/hapi__joi@16 -S

evanshortiss avatar Jul 21 '20 18:07 evanshortiss

Thanks. I tried that, but it breaks joi-extract-type: https://github.com/TCMiranda/joi-extract-type/issues/38

Is there any alternative to avoid redundant type definitions?

danielbrauer avatar Jul 25 '20 15:07 danielbrauer

Unfortunately it appears not with Joi v16. One potential solution is that the 3.x release line of this module could support Joi v15. I think it should be possible to merge the current master branch of this repo into the 3.x branch and set the Joi dev/peer deps in package.json to v15. This could achieve extractType compatibility while a Joi v16 solution is being figured out.

I don't have much time to test that in the next week of so, but perhaps you could fork the module and give it a shot?

evanshortiss avatar Jul 27 '20 11:07 evanshortiss

Thanks, I tried using 3.x with current master (which merged automatically) and the lowered Joi peer dependency. However, the compilation error persists. Was there something in 3.x or master which should address this?

https://github.com/danielbrauer/express-joi-validation/tree/3.x

danielbrauer avatar Jul 28 '20 15:07 danielbrauer

@danielbrauer, works for me. Did you rm -rf node_modules and reinstall?

I started with a fresh project:

# setup new project
cd /tmp
mkdir joi-test
cd joi-test
npm init -f
npm i @hapi/joi@15 @types/hapi__joi@15 danielbrauer/express-joi-validation#3.x -S
npm i express -S && npm i @types/express -D
npm i typescript -D
npx tsc --init

# create index .js
touch index.ts

# paste in your code from this issue and address
# minor tsconfig issues

# compile and run
npx tsc
node index.js

Here's the index.ts I used:

import * as Joi from '@hapi/joi'
import express from 'express'
import {
  ContainerTypes,
  ValidatedRequest,
  ValidatedRequestSchema,
  createValidator
} from 'express-joi-validation'

const app = express()
const validator = createValidator()

const querySchema = Joi.object({
  name: Joi.number().required()
})

interface HelloRequestSchema extends ValidatedRequestSchema {
  [ContainerTypes.Query]: {
    name: number
  }
}

app.get(
  '/hello',
  validator.query(querySchema),
  (req: ValidatedRequest<HelloRequestSchema>, res: express.Response) => { // No overload matches this call
    res.end(`Hello ${req.query.name}!`)
  }
)

app.listen(8080, (err) => {
    if (err) throw err

    console.log('server is listening on 8080')
})

evanshortiss avatar Jul 31 '20 09:07 evanshortiss

@danielbrauer and the resulting package.json was:

{
  "name": "joi-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "@hapi/joi": "^15.1.1",
    "@types/hapi__joi": "^15.0.4",
    "express": "^4.17.1",
    "express-joi-validation": "github:danielbrauer/express-joi-validation#3.x",
    "typescript": "^3.9.7"
  },
  "devDependencies": {
    "@types/express": "^4.17.7"
  }
}

evanshortiss avatar Jul 31 '20 09:07 evanshortiss

Thanks for looking into this further, and for the detailed setup. It appears that the default tsconfig includes skipLibCheck. Removing that flag will expose the error when you run tsc.

danielbrauer avatar Jul 31 '20 13:07 danielbrauer

Good catch. This one isn't too bad to fix. Here's the fix in mainline 3.x.

I think using this might be better:

export interface ExpressJoiError extends Joi.ValidationResult<unknown> {
  type: ContainerTypes
}

If this works could you open a PR against 3.x here and I can create a new 3.x release?

EDIT: this works for me, but just want to get a second verification from you @danielbrauer

evanshortiss avatar Aug 04 '20 09:08 evanshortiss

Facing the same error when using ContainerTypes.Params with a number parameter in it

hqureshi0 avatar Apr 14 '21 12:04 hqureshi0

2024 here, same issue as @hqureshi0 unfortunately

Sufiane avatar Mar 29 '24 14:03 Sufiane

@Sufiane, I don't have much time to maintain this library anymore. If you'd like to open a PR I'd happily release it. I could make some of the folks in this thread maintainers if any of you are willing.

evanshortiss avatar Apr 03 '24 02:04 evanshortiss

@Sufiane, I don't have much time to maintain this library anymore. If you'd like to open a PR I'd happily release it. I could make some of the folks in this thread maintainers if any of you are willing.

truth be told I was just doing a code challenge, I tend to not use express anymore I prefer (and would recommend) going with fastify or if you want a framework to go with nest

Sufiane avatar Apr 05 '24 13:04 Sufiane