Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can I use this with number()? #30

Open
danielbrauer opened this issue Jun 28, 2020 · 15 comments
Open

Can I use this with number()? #30

danielbrauer opened this issue Jun 28, 2020 · 15 comments
Labels

Comments

@danielbrauer
Copy link

danielbrauer commented Jun 28, 2020

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

@evanshortiss
Copy link
Owner

@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
Copy link
Owner

@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

@danielbrauer
Copy link
Author

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 added a commit to danielbrauer/instead that referenced this issue Jul 19, 2020
Running into a compilation error in express-joi-validation
evanshortiss/express-joi-validation#30 (comment)
@evanshortiss
Copy link
Owner

evanshortiss commented Jul 21, 2020

@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

@danielbrauer
Copy link
Author

danielbrauer commented Jul 25, 2020

Thanks. I tried that, but it breaks joi-extract-type: TCMiranda/joi-extract-type#38

Is there any alternative to avoid redundant type definitions?

@evanshortiss
Copy link
Owner

evanshortiss commented Jul 27, 2020

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?

danielbrauer added a commit to danielbrauer/express-joi-validation that referenced this issue Jul 28, 2020
@danielbrauer
Copy link
Author

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

@evanshortiss
Copy link
Owner

@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
Copy link
Owner

@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"
  }
}

@danielbrauer
Copy link
Author

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.

@evanshortiss
Copy link
Owner

evanshortiss commented Aug 4, 2020

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

@hqureshi0
Copy link

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

@Sufiane
Copy link

Sufiane commented Mar 29, 2024

2024 here, same issue as @hqureshi0 unfortunately

@evanshortiss
Copy link
Owner

@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.

@Sufiane
Copy link

Sufiane commented Apr 5, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants