Dependency injection not resolving to the correct singleton instance

[capelski]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

The dependencies declaration order in a service's constructor causes the service to receive a private instance of a depended service instead of the singleton service exported in the corresponding module. It also alters the instantiation order, causing the dependant service to be instantiated before the depended service.

Example

classDiagram

BaseEntityService --> IDatabaseProvider
<<interface>> IDatabaseProvider
AService <-- BService
AService <-- CService
BaseEntityService <|-- AService
BaseEntityService <|-- BService
BaseEntityService <|-- CService

Loading

@Injectable()
export class BService extends BaseEntityService {
  constructor(
    public aService: AService,
    @Inject(DATABASE_PROVIDER) public database: IDatabaseProvider,
  ) {
    super(database);
  }
}

@Injectable()
export class CService extends BaseEntityService {
  constructor(
    @Inject(DATABASE_PROVIDER) public database: IDatabaseProvider,
    public aService: AService,
  ) {
    super(database);
  }
}

Logs

16:49:25.157 Instantiating BService
16:49:25.161 Instantiating AService
16:49:25.162 Instantiating CService
16:49:25.162 Is AService instance the AService singleton instance?
    BService: false
    CService: true

Minimum reproduction code

https://github.com/capelski/nest-dependency-injection-issue

Steps to reproduce

1. npm ci
2. npm run start:dev
3. See logs reflecting the incorrect order in services instantiation

Expected behavior

The dependencies declaration order in a service's constructor doesn't alter the instantiation of the app singleton services.

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

9.0.0

Packages versions

[System Information]
OS Version : macOS Unknown
NodeJS Version : v18.13.0
NPM Version : 8.19.3

[Nest CLI]
Nest CLI Version : 9.0.0

[Nest Platform Information]
platform-express version : 9.0.0
schematics version : 9.0.0
testing version : 9.0.0
common version : 9.0.0
core version : 9.0.0
cli version : 9.0.0

Node.js version

18.13.0

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

No response

[devKangMinHyeok]

I agree. In nestjs, DI (Dependency Injection) is performed based on the type of the arguments, regardless of the order of the arguments. Therefore, even if you change the order of receiving AService and IDatabaseProvider arguments in the BService class constructor, DI should be performed correctly (AService must be a singleton). However, after checking mini reproduction, I am experiencing the same problem in my development environment.

@Injectable()
export class BService extends BaseEntityService {
  constructor(
    public aService: AService,
    @Inject(DATABASE_PROVIDER) public database: IDatabaseProvider,
  ) {
    super(database);
  }
}

import { Controller, Get } from '@nestjs/common';
import { AppService } from './app.service';
import { AService } from './modules/entities/a/a.service';
import { BService } from './modules/entities/b/b.service';
import { CService } from './modules/entities/c/c.service';
import { PlainLogger } from './plain-logger';

const logger = new PlainLogger('AppController');

@Controller()
export class AppController {
  constructor(
    private readonly appService: AppService,
    private readonly aService: AService,
    private readonly bService: BService,
    private readonly cService: CService,
  ) {
    logger.warn(
      `Is ${AService.name} instance the ${AService.name} singleton instance?
    ${BService.name}: ${this.aService === this.bService.aService}
    ${CService.name}: ${this.aService === this.cService.aService}`,
    );
  }

  @Get()
  getHello(): string {
    return this.appService.getHello();
  }
}

In the code above, the aService created by the constructor of the AppController class and the aService created by the constructor of bService are different. This doesn't seem to fit the singleton principle.

I also think this is different from the principles of nestjs we know.

[kamilmysliwiec]

Not sure if I'm following. AService is only instantiated once which can be seen in the logs.

To better understand what's happening in your code, you can add the console.log in the onModuleInit hook of the B service:

Then you'll realize that you're actually injecting 2 DATABASE_PROVIDER providers:

And the reason is that explicit @Inject() always precedes the implicit syntax (inferring deps from types).

Your BService instructs the framework that at the [1] position of the constructor the IDatabaseProvider should be injected, whereas the class you inherit from - BaseEntityService explicitly injects the identical provider at the [0] position. Meaning, now your requested deps array looks as follows: [DATABASE_PROVIDER, DATABASE_PROVIDER] (because explicit > implicit).

If you use the explicit syntax instead, as follows:

Everything will work as expected

[KostyaTretyak]

Meaning, now your requested deps array looks as follows: [DATABASE_PROVIDER, DATABASE_PROVIDER] (because explicit > implicit).

And what in this logic changes the order of dependencies in BService? Why this case works?

@Injectable()
export class BService extends BaseEntityService {
  constructor(
    @Inject(DATABASE_PROVIDER) public database: IDatabaseProvider,
    public aService: AService,
  ) {
    super(database);
  }
}

[kamilmysliwiec]

Because then the explicit injection array is [DATABASE_PROVIDER] (note: no [1] element) since the explicitly injected element is at the same position in the constructor arguments array as in the extended BaseEntityService class.

BaseEntityService [DATABASE_PROVIDER]
BService [DATABASE_PROVIDER]

= [DATABASE_PROVIDER]

while:

BaseEntityService [DATABASE_PROVIDER]
BService [undefined, DATABASE_PROVIDER]

= [DATABASE_PROVIDER, DATABASE_PROVIDER]

[KostyaTretyak]

Incredible logic! @kamilmysliwiec, do you really think that there is nothing to change in this case?

[kamilmysliwiec]

There's actually another issue related to this problem #2581

I know that the current behavior might not be immediately obvious and so as I've mentioned here #2581 (comment) - PRs are more than welcome!

[capelski]

@kamilmysliwiec Ahaa! So the issue is the result of constructor dependencies being declared in a different order in the subclass along with the explicit @Inject() usage in only some of the subclass dependencies. Certainly a tricky one! Thanks for the clarification 😄

[KostyaTretyak]

In fact, this is an obvious bug, it is not a feature. And the fact that this issue is closed without fixing this bug is a bad practice.

[micalevisk]

I guess this was closed in favor of issue #2581

[KostyaTretyak]

#2581 is related to this issue, it's not duplicate of this issue.

[micalevisk]

so you're saying that by fixing that one we wouldn't fix this one too, right?

[KostyaTretyak]

so you're saying that by fixing that one we wouldn't fix this one too, right?

I don't known. Personally, I don't see any relation between #2581 and this issue at all.

[micalevisk]

I think we can trust the author of the code then 😄

[KostyaTretyak]

Unfortunately, I've had the experience of @kamilmysliwiec closing an issue with a bug without actually closing the bug itself. Moreover, this bug related to the security module. So I have reason to assume that closing #2581 may or may not close this issue.