Optional query param with ValidationPipe always converts to NaN

[scopsy]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

We use a validationPipe enabled globally with transform: true. The problem is that we have a query param that is set to be optional, like in the snippet below. And the transform converts it to "NaN" instead of leaving it undefined.

  async getNotificationsFeed(
    @Query('page') page?: number,
    @Query('seen') seen?: boolean
  ) {

seen is always converted to false, even tho I expect it to be undefined or boolean.
and page is converted to NaN when the user is not passing a page query param.

When looking at the ValidationPipe code, it's clear why it's happening since the type conversion always happens without checking for undefined values.

transformPrimitive(value, metadata) {
    if (!metadata.data) {
        // leave top-level query/param objects unmodified
        return value;
    }
    const { type, metatype } = metadata;
    if (type !== 'param' && type !== 'query') {
        return value;
    }
    if (metatype === Boolean) {
        return value === true || value === 'true';
    }
    if (metatype === Number) {
        return +value;
    }
    return value;
}

Minimum reproduction code

https://stackblitz.com/edit/nestjs-typescript-starter-mgxaub?file=src%2Fapp.controller.ts,src%2Fmain.ts

Steps to reproduce

1. Create a new project
2. Install class-validators and class-transform packages
3. Add global validation pipe with transform
4. Add an optional query param with number type
5. Make a request without page query param and the param will be Undefined

Expected behavior

Was expecting it to respect TS annotations and to allow either undefined or a transformed number

Package

• I don't know. Or some 3rd-party package
• @nestjs/common
• @nestjs/core
• @nestjs/microservices
• @nestjs/platform-express
• @nestjs/platform-fastify
• @nestjs/platform-socket.io
• @nestjs/platform-ws
• @nestjs/testing
• @nestjs/websockets
• Other (see below)

Other package

No response

NestJS version

^9.0.0

Packages versions

 "dependencies": {
    "@nestjs/common": "^9.0.0",
    "@nestjs/core": "^9.0.0",
    "@nestjs/platform-express": "^9.0.0",
    "class-transformer": "^0.5.1",
    "class-validator": "^0.13.2",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^7.5.5"
  },
  "devDependencies": {
    "@nestjs/cli": "^9.0.0",
    "@nestjs/schematics": "^9.0.0",
    "@nestjs/testing": "^9.0.0",
    "@types/express": "^4.17.13",
    "@types/jest": "^28.1.4",
    "@types/node": "^18.0.3",
    "@types/supertest": "^2.0.12",
    "@typescript-eslint/eslint-plugin": "^5.30.5",
    "@typescript-eslint/parser": "^5.30.5",
    "eslint": "^8.19.0",
    "eslint-config-prettier": "^8.5.0",
    "eslint-plugin-prettier": "^4.2.1",
    "jest": "^28.1.2",
    "prettier": "^2.7.1",
    "source-map-support": "^0.5.21",
    "supertest": "^6.2.4",
    "ts-jest": "^28.0.5",
    "ts-loader": "^9.3.1",
    "ts-node": "^10.8.2",
    "tsconfig-paths": "^4.0.0",
    "typescript": "^4.7.4"
  },

Node.js version

14.17.6

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

Should we add a check for undefined in the transformPrimitive function? We can also add a new parameter to make sure it doesn't break people code.

I can create a PR for this if that's the right direction to solve this.

[micalevisk]

I might be wrong but I think that's due to reflection limitation. We can't tell if that parameter was declared as optional. So if we add some checking for undefined, both
@Query('foo') param: number
@Query('foo') param?: number
will behave the same, right? 🤔

[scopsy]

Exactly @micalevisk, this will behave just the same for both cases. This is why I thought we could introduce an option and to have another check for undefined variables and just ignore them.

[micalevisk]

as an workaround, you could use @Query() { page, seen }: QueryDTO

[kirisakiken]

@scopsy Have you tried nestjs parsers for primitive types?

  async getNotificationsFeed(
    @Query('page', ParseIntPipe) page?: number,
    @Query('seen', ParseBoolPipe) seen?: boolean
  ) {

[scopsy]

@kirisakiken not yet, will try to check it out 🙏

[kamilmysliwiec]

Let's track this here #10953

[Hareloo]

This isn't actually solved.. Passing no value to a query param of type number still results in NaN instead of undefined...

[micalevisk]

@Hareloo please open another issue instead

[thw0rted]

This should be fixed when #12893 is merged. Anyone finding this issue may also be interested in #13559 which I just opened.