@Query param being generated as mandatory when defined as possibly undefined.

jsanson98 commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

When setting a @Query param in a @Get and defining that param as possibly undefined, the args type for that query are created as if that field is mandatory.

Ex: @Query('skip') skip?: number

Arg created as:

type FindAllApiArg = {
    skip: number;
}

A fix is to add: @ApiQuery({ name: 'skip', required: false, type: Number }). But it's not desired.

Minimum reproduction code

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

Steps to reproduce

No response

Expected behavior

The arg should be created as:

type FindAllApiArg = {
    skip?: number | undefined;
}

Package

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

Other package

No response

NestJS version

No response

Packages versions

[System Information]
OS Version     : macOS Big Sur
NodeJS Version : v18.13.0
NPM Version    : 8.19.3 

[Nest CLI]
Nest CLI Version : 9.3.0 

[Nest Platform Information]
class-transformer version : 0.4.0
platform-express version  : 9.3.9
class-validator version   : 0.13.4
event-emitter version     : 1.4.1
serve-static version      : 3.0.1
schematics version        : 9.0.4
passport version          : 9.0.3
schedule version          : 2.2.0
terminus version          : 9.2.1
swagger version           : 6.2.1
typeorm version           : 9.0.1
testing version           : 9.3.9
common version            : 9.3.9
core version              : 9.3.9
cli version               : 9.3.0

Node.js version

No response

In which operating systems have you tested?

[ ] macOS
[ ] Windows
[ ] Linux

Other

No response

micalevisk commented 1 year ago

That is a known limitation of typescript metadata reflection :/

chriskuech commented 1 year ago

@micalevisk can you provide any more insight? If ? doesn't work, shouldn't | undefined still work? Are there specific limitations around method argument types that don't apply to class property types?

micalevisk commented 1 year ago

@chriskuech at runtime, there's no way to infer that that parameter is optional.

Using | undefined won't even report that parameterto th just because x | undefined is a type notation. And type notations are erased by typescript compiler. The same lies to | null

I don't recall where you can learn more those limitations of reflect-metadata, sorry. But isn't related with nestjs itself.

nestjs / nest