onModuleInit() not called in correct order

[clkamp]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

A module may be initialized before its dependencies are initialized.

Suspected culprit is the skip condition at

nest/packages/core/scanner.ts

Line 394 in 7843db3

if (!moduleRef || modulesStack.includes(moduleRef)) {

While the || modulesStack.includes(moduleRef) protects against infinite loops for cyclic dependencies, it also gives wrong results in some cases, because the moduleRef may have entered the recursive function with a different(higher) value for distance than during its last evaluation. This is not necessarily a cyclic dependency.

Minimum reproduction code

https://github.com/amadeusjposchmann/nestDistanceExample

Steps to reproduce

1. npm ci
2. npm run start

Expected behavior

YSerivce's onModuleInit() should be called before XService's onModuleInit() is called.

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.4.5

Packages versions

10.4.5

Node.js version

No response

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

Consider a module A, depending on X, depending on Y.

Your root/application module depends on modules X and A, giving you these chains:

R->X->Y
R->A->X->Y

Evaluating the first chain, the scanner assigns distances X:1, Y:2 .
Evalutaing the second chain, the scanner updates to A:1, X:2, Y:2 .
X and Y end up with the same distances, despite X depending on Y, because the scanner skips the second X->Y reference. They may even end up in reverse order, if there would be another module inbetween ( i.e. A->B->C->X->Y )

For testing purposes, we've removed the suspect culprit condition in a testing setup and it's working as expected in our project where the issue surfaced. Will create a PR as proposal.

This was also previously reported as #11672, but was not fixed and closed, because the simple proposed fix would not work for cyclic dependencies.

[kamilmysliwiec]

Let's track this here #14097