Swagger doesn't show params from a DTO if it uses extends PickType

YakovL commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

I've implemented a "rename project" endpoint which looks like this:

@Patch('/:projectId/name/:newName')
async rename(@Param() params: UpdateProjectNameParamsDto): Promise<Project> {
  return this.projectsService.rename(params.projectId, params.newName);
}

and UpdateProjectNameParamsDto looks like this:

export class UpdateProjectNameParamsDto {
  @ApiProperty({
    description: 'Internal Id of project',
    example: '10fa8784-7006-499d-b97a-a406901df2b8',
  })
  @IsNotEmpty()
  @IsUUID()
  projectId: string;

  @ApiProperty({
    description: 'The name that project should be renamed into',
    example: 'The Best Project',
  })
  @IsNotEmpty()
  @IsString()
  newName: string;
}

This worked fine (the endpoint works, swagger shows what's expected ..well, except for swagger doesn't validate IsUUID and if I send a string that's not UUID, I'm getting a 500 error instead of.. 400? and a helpful error message; but this is not the main issue).

I've noticed that we already have

export class ProjectDto implements Project {
  @ApiProperty({
    description: 'Internal Id of project',
    example: '10fa8784-7006-499d-b97a-a406901df2b8',
  })
  @IsNotEmpty()
  @IsUUID()
  id: string;

  @IsNotEmpty()
  @IsString()
  @ApiProperty({ description: 'Name of project', example: 'name' })
  name: string;

  ...
}

So, I decided to reuse those and tried instead (looking at the docs):

  export class UpdateProjectNameParamsDto extends PickType(ProjectDto, [
    'id',
    'name',
  ] as const) {}

and

@Patch('/:id/name/:name')
async rename(@Param() params: UpdateProjectNameParamsDto): Promise<Project> {
  return this.projectsService.rename(params.id, params.name);
}

The result is.. unexptected: while the same request still works as previously, Swagger now shows that the endpoint has no params!

Minimum reproduction code

https://github.com/YakovL/nest-typescript-starter/tree/swagger-dto-mcve

Steps to reproduce

Note: in the mcve, I've used Get instead of Patch, just for simplicity; the bug affects Get, too.

git clone https://github.com/YakovL/nest-typescript-starter.git, cd nest-typescript-starter
git checkout swagger-dto-mcve
npm i, npm run start:dev
open http://localhost:3000/someId/name/someName and http://localhost:3000/api#/default/AppController_rename
optionally, compare the results with those before refactor: git checkout a09d1c3

Expected behavior

Both for HEAD and for a09d1c3, the endpoint responds with a line that tells someId and someName were passed, and swagger shows that the endpoint has 2 params.

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
[X] Other (see below)

Other package

May be @nestjs/swagger, @nestjs/mapped-types; less likely class-validator

NestJS version

10.2.6

Packages versions

  "dependencies": {
    "@nestjs/common": "^10.0.0",
    "@nestjs/core": "^10.0.0",
    "@nestjs/platform-express": "^10.0.0",
    "class-validator": "^0.14.0",
    "reflect-metadata": "^0.1.13",
    "rxjs": "^7.8.1"
  },
  "devDependencies": {
    "@nestjs/cli": "^10.0.1",
    "@nestjs/schematics": "^10.0.1",
    "@nestjs/swagger": "^7.1.14",
    "@nestjs/testing": "^10.0.0",
    "@swc/cli": "^0.1.62",
    "@swc/core": "^1.3.64",
    "@types/express": "^4.17.17",
    "@types/jest": "^29.5.2",
    "@types/node": "^20.3.1",
    "@types/supertest": "^2.0.12",
    "@typescript-eslint/eslint-plugin": "^5.59.11",
    "@typescript-eslint/parser": "^5.59.11",
    "eslint": "^8.42.0",
    "eslint-config-prettier": "^8.8.0",
    "eslint-plugin-prettier": "^4.2.1",
    "jest": "^29.5.0",
    "prettier": "^2.8.8",
    "source-map-support": "^0.5.21",
    "supertest": "^6.3.3",
    "ts-jest": "^29.1.0",
    "ts-loader": "^9.4.3",
    "ts-node": "^10.9.1",
    "tsconfig-paths": "^4.2.0",
    "typescript": "^5.1.3"
  },

Node.js version

18.16.0

In which operating systems have you tested?

[ ] macOS
[X] Windows
[ ] Linux

Other

No response

jmcdo29 commented 1 year ago

1) swagger doesn't validate the DTO. That comes from the user of the ValidationPipe and the class-validator decorators 2) you should use PickType from @nestjs/swagger instead of @nestjs/mapped-types to ensure the swagger metadata is propagated as expected

YakovL commented 1 year ago

@jmcdo29 thanks, switching to PickType from @nestjs/swagger helped (I've pushed the fix ae8d224 to the same repo).

In the MCVE repo, there was also no global validation enabled, I've added it in a separate commit (2956d1b).

nestjs / nest