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

Dates objects are automatically transformed with the ValdiationPipe and at least one validator option #12647

Closed
3 of 15 tasks
jmcdo29 opened this issue Oct 24, 2023 · 1 comment
Labels
needs triage This issue has not been looked into

Comments

@jmcdo29
Copy link
Member

jmcdo29 commented Oct 24, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

This was something interesting brought up in a Discord message.

When using the ValidaitonPipe, with transform: false and defining whitelist: false (or any other variant of) along with @Query('date') date: Date, when accessing date in the route handler, it would come back as a Date object, rather than the expected string. When removing the whitelist option altogether, the date variable was correctly a string.

Minimum reproduction code

https://github.com/jmcdo29/nest-whitelist-date-validation

Steps to reproduce

  1. pnpm i (or whatever package manager you want
  2. pnpm start:dev
  3. curl http://localhost:3000?date=2023-10-24T:10:00:00
    a. alternatively, run pnpm test:e2e
  4. Notice the logs
    a. or test failures

Expected behavior

I believe that we don't want to run validations on Date class properties if they're single values, like when accessed via @Query('date'), @Param('date'), @Body('date'), etc. The fix for this is to simply add Date to the toValidate array of class types that we don't immediately run validations for.

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

10.2.7

Packages versions

 _   _             _      ___  _____  _____  _     _____
| \ | |           | |    |_  |/  ___|/  __ \| |   |_   _|
|  \| |  ___  ___ | |_     | |\ `--. | /  \/| |     | |
| . ` | / _ \/ __|| __|    | | `--. \| |    | |     | |
| |\  ||  __/\__ \| |_ /\__/ //\__/ /| \__/\| |_____| |_
\_| \_/ \___||___/ \__|\____/ \____/  \____/\_____/\___/


[System Information]
OS Version     : Linux 6.5
NodeJS Version : v20.8.0
PNPM Version    : 8.7.1 

[Nest CLI]
Nest CLI Version : 10.2.0 

[Nest Platform Information]
platform-express version : 10.2.7
schematics version       : 10.0.2
testing version          : 10.2.7
common version           : 10.2.7
core version             : 10.2.7
cli version              : 10.2.0

Node.js version

20.8.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

Happy to open the PR for this, just wanted to verify it before I did

@jmcdo29 jmcdo29 added the needs triage This issue has not been looked into label Oct 24, 2023
@kamilmysliwiec
Copy link
Member

If you could submit a PR for this that would be great 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

2 participants