nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
67.63k stars 7.62k forks source link

Global custom validator with imports from two different modules is only triggered in one of the two modules #12847

Closed SakisTsalk closed 11 months ago

SakisTsalk commented 11 months ago

Is there an existing issue for this?

Current behavior

If the same validator is being used in two different modules, and Dto Objects from both of the modules are being used inside the validator, the validator only triggers in one of the two modules.

Minimum reproduction code

https://github.com/SakisTsalk/nestjs-custom-validators-issue

Steps to reproduce

  1. install dependencies and start application yarn install yarn start:dev
  2. Make two POST requests curl --location 'http://localhost:3000/dogs/create' \ --header 'Content-Type: application/json' \ --data '{ "dog": { "name": "kittie" } }'

curl --location 'http://localhost:3000/cats/create' \ --header 'Content-Type: application/json' \ --data '{ "cat": { "name": "kittie" } }'

Expected behavior

The validate function of @CustomValidatorPipe should be triggered in both API calls.

But it is only triggered for http://localhost:3000/cats/create

Package

Other package

class-validator, class-transformer

NestJS version

10.2.3

Packages versions

{
  "name": "nestjs-custom-validators-issue",
  "version": "0.0.1",
  "description": "",
  "author": "",
  "private": true,
  "license": "UNLICENSED",
  "scripts": {
    "prebuild": "rimraf dist",
    "build": "nest build",
    "format": "prettier --write \\\"src/**/*.ts\\\" \\\"test/**/*.ts\\\"",
    "start": "nest start",
    "start:dev": "nest start --watch",
    "start:debug": "nest start --debug --watch",
    "start:prod": "node dist/main",
    "lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix",
    "test": "jest",
    "test:watch": "jest --watch",
    "test:cov": "jest --coverage",
    "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
    "test:e2e": "jest --config test/jest-e2e.json"
  },
  "dependencies": {
    "@nestjs/common": "^10.2.5",
    "@nestjs/core": "^10.2.3",
    "@nestjs/platform-express": "^10.2.3",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^7.2.0",
    "class-validator": "0.14.0",
    "class-transformer": "^0.5.1"
  },
  "devDependencies": {
    "@nestjs/cli": "^8.0.0",
    "@nestjs/schematics": "^8.0.0",
    "@nestjs/testing": "^8.0.0",
    "@types/express": "^4.17.13",
    "@types/jest": "27.0.2",
    "@types/node": "^16.0.0",
    "@types/supertest": "^2.0.11",
    "@typescript-eslint/eslint-plugin": "^5.0.0",
    "@typescript-eslint/parser": "^5.0.0",
    "eslint": "^8.0.1",
    "eslint-config-prettier": "^8.3.0",
    "eslint-plugin-prettier": "^4.0.0",
    "jest": "^27.2.5",
    "prettier": "^2.3.2",
    "source-map-support": "^0.5.20",
    "supertest": "^6.1.3",
    "ts-jest": "^27.0.3",
    "ts-loader": "^9.2.3",
    "ts-node": "^10.0.0",
    "tsconfig-paths": "^3.10.1",
    "typescript": "^4.3.5"
  },
  "jest": {
    "moduleFileExtensions": [
      "js",
      "json",
      "ts"
    ],
    "rootDir": "src",
    "testRegex": ".*\\.spec\\.ts$",
    "transform": {
      "^.+\\.(t|j)s$": "ts-jest"
    },
    "collectCoverageFrom": [
      "**/*.(t|j)s"
    ],
    "coverageDirectory": "../coverage",
    "testEnvironment": "node"
  }
}

Node.js version

18.13.0

In which operating systems have you tested?

Other

No response

jmcdo29 commented 11 months ago

So, I think the issue here is that there's circular imports between the models and the validator, and you're honestly just getting lucky that it's working for the cats dto. These kinds of circular dependencies aren't things Nest knows about because it doesn't have any impact on the startup of the server, and would be more in class-vadliators domain to keep track of. You can try to open an issue with them about it and see if they have any suggestions.

What you can technically do is access .constructor.name of validationArguments.object, but it's not type safe so YMMV with that.

Generally speaking, a better option, would probably be to use a dedicated pipe that does this validation rather than forcing class-validator to work with it like this

SakisTsalk commented 11 months ago

@jmcdo29 first of all thanks for taking a look at the issue and the advice.

Yeah I am aware of using .constructor.name I can also move the dtos to a higher tree level module as well.

I am more looking into prevending similar issues happening in the future since one can not always be too careful. In my case it took me hours to figure out why the validator was not working. So ideally an error thrown would save me all this time of debugging.

Opened an issue in class-validator as well https://github.com/typestack/class-validator/issues/2320