Support using `@Inject()` decorator factory with no args on constructor-based injections

[micalevisk]

trafficstars

Is there an existing issue that is already proposing this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe it

If we use @Inject() on constructor, Nest will think that that is due to some circular (acting the same as @Inject(undefined), basically). While we should use @Inject() on property-based injections.

For example:

import { Module, Inject } from '@nestjs/common';

class Foo {}

@Module({
  providers: [Foo],
})
export class AppModule {
  @Inject() foo1: Foo

  constructor(
    @Inject(Foo) readonly foo2: Foo,
    @Inject() readonly foo3: Foo, // Fails
  ) {}

  onModuleInit() {
    console.log('foo1 instanceof Foo:', this.foo1 instanceof Foo)
    console.log('foo1 === foo2:', this.foo1 === this.foo2)
    console.log('foo1 === foo3:', this.foo1 === this.foo3)
  }
}

as of now, we'll see the following error:

[Nest] 286235  - 04/12/2024, 12:15:25 PM   ERROR [ExceptionHandler] Nest can't resolve dependencies of the AppModule (Foo, ?). Please make sure that the argument dependency at index [1] is available in the AppModule context.

Potential solutions:
- Is AppModule a valid NestJS module?
- If dependency is a provider, is it part of the current AppModule?
- If dependency is exported from a separate @Module, is that module imported within AppModule?
  @Module({
    imports: [ /* the Module containing dependency */ ]
  })

Describe the solution you'd like

I think that it will be less confusing if we start supporting @Inject() on constructor-based injection as well. Those @Inject() calls just need to be 'ignored', so people won't complain on why they're seen such error. Working the same as not having @Inject()

I managed to change the following code in order to support that:

https://github.com/nestjs/nest/blob/6f119a7fdd46718327d3e1f4cb0bdc5d0ac7531b/packages/common/decorators/core/inject.decorator.ts#L36-L40

to:

// ...
const injectCallHasArguments = arguments.length > 0
return (target, key, index) => {
	let type = (token || Reflect.getMetadata('design:type', target, key));
    if (!type && !injectCallHasArguments) {
      type = Reflect.getMetadata('design:paramtypes', target, key)?.[index];
    }
// ...

so now I'm seen this output instead of that error:

but I'm not sure if that's the right way to implement such feature

Teachability, documentation, adoption, migration strategy

N/A

What is the motivation / use case for changing the behavior?

• discord questions (from newbies) on why the injecting isn't working as people expect
• make the property-based injection and constructor-based injection more alike (ie., an uniform API), so we can move from property-based injection to constructor-based and vice-versa without thinking too much. This also should increase the learning curve -- learn once, apply anywhere