Hooks in global module which imported more than twice might called multiple times

5d-jh commented 6 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

If Dynamic module once imported as intended(a), and just imported as just class token(b), dynamic module's lifecycle hook called twice. Need to deduplicate modules by metatype for this case.

class ModuleA implements OnApplicationShutdown {
  static forRoot() {
    global: true,
    module: ModuleA
  }

  onApplicationShutdown() {
    console.log('shutting down....')
  }
}

class ModuleB {
  static forRoot() {
    global: true,
    module: ModuleB,
    imports: [ModuleA], // <-- (b)
  }
}

class ModuleC {
  static register() {
    global: true,
    module: ModuleC
    imports: [ModuleA.forRoot()] // <-- (a)
  }
}

@Module({
  imports: [ModuleB.forRoot(), ModuleC.forRoot()]
})
class AppModule {}

const app = NestFactory.create(AppModule)
await app.init();
await app.close();

In console

shutting down....
shutting down....

Minimum reproduction code

Steps to reproduce

No response

Expected behavior

In console

shutting down....

Package

[ ] I don't know. Or some 3rd-party package
[ ] code>@nestjs/common</code
[X] 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

10.3.2

Packages versions

{
  "name": "nest-typescript-starter",
  "private": true,
  "version": "1.0.0",
  "description": "Nest TypeScript starter repository",
  "license": "MIT",
  "scripts": {
    "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/jest/bin/jest --runInBand",
    "test:e2e": "jest --config ./test/jest-e2e.json"
  },
  "dependencies": {
    "@nestjs/common": "^10.3.2",
    "@nestjs/core": "^10.3.2",
    "@nestjs/platform-express": "^10.3.2",
    "reflect-metadata": "^0.2.1",
    "rxjs": "^7.8.1"
  },
  "devDependencies": {
    "@nestjs/cli": "^10.3.1",
    "@nestjs/schematics": "^10.1.0",
    "@nestjs/testing": "^10.3.2",
    "@swc/cli": "^0.3.9",
    "@swc/core": "^1.4.0",
    "@types/express": "^4.17.21",
    "@types/jest": "^29.5.12",
    "@types/node": "^20.11.16",
    "@types/supertest": "^6.0.2",
    "@typescript-eslint/eslint-plugin": "^6.21.0",
    "@typescript-eslint/parser": "^6.21.0",
    "eslint": "^8.56.0",
    "eslint-config-prettier": "^9.1.0",
    "eslint-plugin-prettier": "^5.1.3",
    "jest": "^29.7.0",
    "prettier": "^3.2.5",
    "source-map-support": "^0.5.21",
    "supertest": "^6.3.4",
    "ts-jest": "^29.1.2",
    "ts-loader": "^9.5.1",
    "ts-node": "^10.9.2",
    "tsconfig-paths": "^4.2.0",
    "typescript": "^5.3.3"
  },
  "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

20.9.0

In which operating systems have you tested?

[X] macOS
[ ] Windows
[X] Linux

Other

No response

micalevisk commented 6 months ago

technically, ModuleA and ModuleA.forRoot() aren't the same thing so you'll have two different instances of ModuleA class in this case, which is why the hooks were called twice. I might be wrong but I believe that this is the desired behavior

kamilmysliwiec commented 6 months ago

Thank you for taking the time to submit your report! From the looks of it, this could be better discussed on our Discord. If you haven't already, please join here and send a new post in the #⁠ 🐈 nestjs-help forum. Make sure to include a link to this issue, so you don't need to write it all again. We have a large community of helpful members, who will assist you in getting this to work.

nestjs / nest