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.62k stars 7.61k forks source link

Allow useClass with dynamic provider injection #4476

Closed kierans closed 4 years ago

kierans commented 4 years ago

Feature Request

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

Nest currently has a useFactory option to allow Custom Providers to dynamically specify their object graph. This is useful when wanting to use a named instance (via a string token) as an argument, when using Dynamic Modules, etc. However when wanting to instantiate a class that is not @Injectable() currently the only way to that is via a factory function eg:

class MyClass {
  constructor(a: Something, b: AnotherThing) { ... }
}

// in a @Module()
{
  provide: MyClass
  useFactory: (a, b) => new MyClass(a, b),
  inject: [
    Something,
    AN_INSTANCE_OF_ANOTHER_THING
  ]
}

The factory function is really superfluous as it's a wrapper around the constructor.

Currently if useClass is used because the inject array is not used the constructor arguments are undefined (see this example)

Describe the solution you'd like

Have useClass also use the inject array for dynamic (non @Injectable()) objects so that arguments can be injected into the object constructor and eliminating the need for a wrapper factory function.

Teachability, Documentation, Adoption, Migration Strategy

  1. Update the docs to further explain the difference between dynamic providers and providers that are coupled together through the use of the @Injectable and @Inject decorators.
  2. Document how useClass can be used to instantiate a dynamic provider, rather than having to use a factory function that wraps the new call.

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

Having extra factory functions leads to duplication of code as if the constructor changes, so does the function signature.


This is related to #219 which had similar thoughts/ideas but didn't seem to go anywhere. Maybe it's something about Typescript I'm not understanding, but if you can type arguments in a factory function, and have the inject array why can't we use the class constructor?

kamilmysliwiec commented 4 years ago

Have useClass also use the inject array for dynamic (non @Injectable()) objects so that arguments can be injected into the object constructor and eliminating the need for a wrapper factory function.

Why can't you use @Injectable()?

kierans commented 4 years ago

Because I'm using string tokens to identify/name some of the arguments that are to be injected. So I can't use @Injectable because Nest can't identify the dependencies (unless I put @Inject(TOKEN_NAME) into the parameters of the injectee's constructor, but that doesn't work for my use case as I reuse a lot of classes with different object relationships and @Inject couples instances together and IMO violates IoC)

kamilmysliwiec commented 4 years ago

(unless I put @Inject(TOKEN_NAME) into the parameters of the injectee's constructor, but that doesn't work for my use case as I reuse a lot of classes with different object relationships and @Inject couples instances together and IMO violates IoC)

What do you mean by "that doesn't work for my use case as I reuse a lot of classes with different object relationships and @Inject couples instances together and IMO violates IoC"? How does this violate IoC? What prevents you from reusing a class which uses @Inject()? How @Inject() couples instances together?

jmcdo29 commented 4 years ago

Is there a reason you couldn't set up the injection token as a custom provider, and swap out what values/classes/factories should be used when Nest sees that injection token in different modules?

kierans commented 4 years ago

Is there a reason you couldn't set up the injection token as a custom provider, and swap out what values/classes/factories should be used when Nest sees that injection token in different modules?

I'm sorry @jmcdo29 but I'm not following your thinking here. This feature request is about how to have Nest instantiate an object that has a dependency that is identified by a string token. Perhaps an example would help me understand πŸ˜„

jmcdo29 commented 4 years ago

I'll try to get an example tonight to show what I'm thinking

kierans commented 4 years ago

What do you mean by "that doesn't work for my use case as I reuse a lot of classes with different object relationships and @Inject couples instances together and IMO violates IoC"? How does this violate IoC? What prevents you from reusing a class which uses @Inject()? How @Inject() couples instances together?

@kamilmysliwiec As far as I understand how Nest's injection system works, if you're using static wiring you're using the decorators to tell Nest what relationships you want created between object instances.

@Injectable()
class A {}

@Injectable()
class B {
  /*
   * Nest uses the class as the token to instantiate an `A`
   * and provide it as an argument to `a`. Manually specifying
   * an instance to inject here via @Inject() is superfluous.
   */
  constructor(a: A) {}
}

However what happens when we want an instance of an interface, or I want to create another instance of an injectable class?

interface I {}

class Y implements I {}
class Z implements I {}

@Injectable()
class C {
  /*
   * What gets injected here, a `Y` or a `Z`?!
   */
  constructor(x: I) {}
}

How do I provide the relationship data to C? One way is via @Inject()

@Injectable()
class C {
  /*
   * Now we know that `x` is a `Y`
   */
  constructor(@Inject(Y) x: I) {}
}

However C is now coupled to Y. I will never get a C where x is a Z from the container. This is why I believe that @Inject() violates IoC. Because you've still coupled two types together, but using a container instead of using concrete types directly. The outcome is the same. The point of IoC is that you rely on the type contracts only. However even if one disagrees with me on the idea that @Inject() violates the IoC principle, it definitely violates the Open Closed Principle as if I want to change the behaviour of C (which uses an instance of I) I have to modify the source of C)

So what if I want two Cs? The only way is with dynamic provider wiring (and removing @Injectable from C). However because only useFactory utilises the inject array, I have to use wrapper functions to instantiate the objects.

providers: [
  {
    provide: C_1
    useFactory: (x) => new C(x),
    inject: [
      Y
    ]
  },
  {
    provide: C_2
    useFactory: (x) => new C(x),
    inject: [
      Z
    ]
  }
]

What the goal of this issue is, is to allow useClass to use inject so that I don't have to write wrapper factory functions. I would like to write the above as:

providers: [
  {
    provide: C_1
    useClass: C,
    inject: [
      Y
    ]
  },
  {
    provide: C_2
    useClass: C,
    inject: [
      Z
    ]
  }
]

Where I have real use cases is that my application utilises a lot of third party services using the OAuth 2 workflow for authorisation. So I have written classes that abstract away the OAuth 2 integration logic from the rest of my application and then I instantiate the objects with different configuration (constructor arguments) using dynamic provider wiring. For example C is a class which calls a third party to sync some resource data and I is a "transformer" interface that knows how to transform the third party data structure into my application domain model. The logic of "call the third party and manage all the authorisation token logic" is the same regardless of the third party, all that's different is the resource data.

As I integrate more third party services I only have to update one module and the bulk of my integration with that third party is complete. I have found that I have to write a lot of wrapper factory functions that provide no real value but are necessary to be able to inject different dependencies.

I hope that clarifies why I have raised this Feature Request :smile:

kamilmysliwiec commented 4 years ago

However C is now coupled to Y. I will never get a C where x is a Z from the container. This is why I believe that @Inject() violates IoC. Because you've still coupled two types together, but using a container instead of using concrete types directly. The outcome is the same.

This isn't true. As @jmcdo29 suggested, you can use an injection token. This token can be a string, symbol, class, abstract class, or even an instance of your "custom token class". Then, you can register different classes/objects (depending on your use-case) per different scope (e.g., module/library). Note that C is not coupled to any type in this case.

Where I have real use cases is that my application utilises a lot of third party services using the OAuth 2 workflow for authorisation. So I have written classes that abstract away the OAuth 2 integration logic from the rest of my application and then I instantiate the objects with different configuration (constructor arguments) using dynamic provider wiring. For example C is a class which calls a third party to sync some resource data and I is a "transformer" interface that knows how to transform the third party data structure into my application domain model. The logic of "call the third party and manage all the authorisation token logic" is the same regardless of the third party, all that's different is the resource data. As I integrate more third party services I only have to update one module and the bulk of my integration with that third party is complete. I have found that I have to write a lot of wrapper factory functions that provide no real value but are necessary to be able to inject different dependencies.

Same as above. You can do it with injection tokens without having wrapper factory functions.

However even if one disagrees with me on the idea that @Inject() violates the IoC principle, it definitely violates the Open Closed Principle as if I want to change the behaviour of C (which uses an instance of I) I have to modify the source of C)

It doesn't. Same as above, as long as you define a token, you will never have to modify the source of C.

Anyways, even if for some reason you still don't want to use the @Inject decorator, there's still a way to not use it at all.

In C++, we use virtual methods (= 0) to define interfaces. In TypeScript, you don't necessarily have to use "interfaces" (which are wiped out from the JS bundle so we can't refer them at runtime) either. Instead, you can use abstract classes:

abstract class I {
  abstract i(): number;
}

class C implements I {
  i(): number {
    throw new Error("Method not implemented.");
  }
}

As you can see, C implements I (which is not an interface, but an abstract class which acts as an interface - which is completely valid and works equivalently). Since classes exist at runtime (JS functions), you can use them as injection tokens. Furthermore, in this case, you don't even have to use @Inject() anymore.

@Injectable()
class Provider {

  constructor(x: I) {} // where `I` is an abstract class
}

Then, depending on the scope, you can provide a different implementation:

{
   provide: I,
   useClass: C,
}

And in another module:

{
   provide: I,
   useClass: A, // a different class which also implements I
}
jmcdo29 commented 4 years ago

@kierans here's a small repository I made to show an example of how you can swap out the tokens so long as you're using string tokens with @Inject(). The readme has some good information on it, but I advise you look into the code to get a deeper understanding.

kierans commented 4 years ago

Thanks @jmcdo29 πŸ‘

kierans commented 4 years ago

This isn't true. As @jmcdo29 suggested, you can use an injection token. This token can be a string, symbol, class, abstract class, or even an instance of your "custom token class". Then, you can register different classes/objects (depending on your use-case) per different scope (e.g., module/library). Note that C is not coupled to any type in this case.

I must admit, I hadn't thought of this token indirection (whether by using string tokens, or by using class tokens with an abstract base class)

Then, depending on the scope, you can provide a different implementation:

{
   provide: I,
   useClass: C,
}

And in another module:

{
   provide: I,
   useClass: A, // a different class which also implements I
}

However the example above and the top level quote assume that I want to only have different implementations, or a single instance of a class per module.

For example I have a Providers module (I know, confusing name, but it fits my business domain) that exports the services relating to using my third party OAuth 2 services. Going to my description in a previous comment about how I have multiple instances of the same classes but with different constructor arguments, using token indirection still unfortunately isn't a viable solution to me. I still have to use factories at this point.

I think it comes down to the fact that in my view, useClass is saying "use this constructor function". So if I have used @Inject() in the constructor to provide metadata, or use the inject array the dependencies should be resolved and injected.

kierans commented 4 years ago

@kamilmysliwiec Can you please reopen this as you closed it while I was asleep (different timezone) and due to being consumed with starting another contract yesterday (my time) I was delayed in responding to this. I would still like to see this feature implemented.

jmcdo29 commented 4 years ago

I'm still not sure why of you can use a factory you can't use a class. If you can share the code you're working with I'm pretty sure we could get something working

kierans commented 4 years ago

@jmcdo29 Here's a cut down idea of what my application is doing (written in Vim off the top of my head, please forgive any syntax errors).

interface ProviderTransformer<D, M> {
  transform(dto: D): M
}

@Injectable()
class GoogleCalendarTransformer implements ProviderTransformer<GoogleCalendarDTO, CalendarModel> {
  transform(dto: GoogleCalendarDTO): CalendarModel {
    // do work
  } 
}

@Injectable()
class ExchangeCalendarTransformer implements ProviderTransformer<ExchangeCalendarDTO, CalendarModel> {
  transform(dto: ExchangeCalendarDTO): CalendarModel {
    // do work
  } 
}

/*
 * This class does the work to the call the third party calendar service,
 * managing OAuth 2 access tokens, and other security necessities.
 */
class ThirdPartyCalendarService {
  constructor(private readonly transformer: ProviderTransformer<any, CalendarModel>) {

  }

  syncCalendars() {
    calanderData = this.fetchCalendarData();

    calendarModel: CalendarModel = this.transformer.transform(calendarData);

    return calendarModel;
  }
}

@Module({
  providers: [
    {
      provides: "googleCalendarProvider",
      useFactory: (t) => new ThirdPartyCalendarService(t),
      inject: [
        GoogleCalendarTransformer 
      ]
    },
    {
      provides: "exchangeCalendarProvider",
      useFactory: (t) => new ThirdPartyCalendarService(t),
      inject: [
        ExchangeCalendarTransformer 
      ]
    }
  ],
  exports: [
    "googleCalendarProvider",
    "exchangeCalendarProvider"
  ]
})
class ProvidersModule {

}

I've followed good SOLID design principles in the development of my application and separated the concerns. The ThirdPartyCalendarService knows how to integrate with the third party calendar services according to the OAuth 2 specification. The ProviderTransformer does it's job and all is well. The code is modular, reusable, testable and all round shiny.

However because useClass doesn't use the inject array I have to use factory functions that don't offer value. Whenever I change a classes constructor signature I have to update every factory function (or at least the one that I pass by reference to useFactory).

While the idea of token indirection was a good one (and I'm glad to have learned it), in this situation I don't see how it accommodates multiple objects of one class in the same module.

This feature request is about being about to use useClass with dynamic providers which I think will be a helpful feature to developers ie:

{
  provides: "exchangeCalendarProvider",
  useClass: ThirdPartyCalendarService,
  inject: [
    ExchangeCalendarTransformer 
  ]
}

Or failing that can the class constructor be passed by reference and called with new ie:

useFactory: ThirdPartyCalendarService
jmcdo29 commented 4 years ago

@kierans using what you've provided above, the follow solution should work as well, though maybe not as elegantly as you would want:


@Injectable()
export class ThirdPartyCalendarService {
  constructor(@Inject('CalendarTransformer') private readonly transformer: ProviderTransformer<any, CalendarModel>) {}
}

@Module({
  providers: [
    {
      provide: 'CalendarTransformer',
      useClass: GoogleCalendarTransformer,
    },
    {
      provide: 'googleCalendarProvider',
      useClass: ThirdPartyCalendarService,
    },
  ],
  exports: [ 'googleCalendarProvider'],
})
export class GoogleCalendarModule {}

@Module(
  providers: [
    {
      provide: 'CalendarTransformer',
      useClass: ExchangeCalendarTransformer,
    },
    {
      provide: 'exchangeCalendarProvider',
      useClass: ThirdPartyCalendarService,
    },
  ],
  exports: [ 'exchangeCalendarProvider'],
)
export class ExchangeCalendarModule {}

@Module({
  imports: [ExchangeCalendarModule, GoogleCalendarModule],
  exports: [ExchangeCalendarModule, GoogleCalendarModule]
})
export class ProvidersModule {}

Something to keep in mind, is that while this introduces two new modules to the system, it also ensures that your CalendarTransformer is singleton scoped between the two modules (or however many you have). This holds true with factories due to the fact that the factory itself acts like a transient module, creating the provider and exporting it for use.

kamilmysliwiec commented 4 years ago

A modified version with fewer strings and decorators:

abstract class ProviderTransformer<D, M> {
  abstract transform(dto: D): M;
}

@Injectable()
export class ThirdPartyCalendarService {
  constructor(private readonly transformer: ProviderTransformer<any, CalendarModel>) {}
}

@Module({
  providers: [
    {
      provide: ProviderTransformer,
      useClass: GoogleCalendarTransformer,
    },
    {
      provide: 'googleCalendarProvider',
      useClass: ThirdPartyCalendarService,
    },
  ],
  exports: ['googleCalendarProvider'],
})
export class GoogleCalendarModule {}

@Module(
  providers: [
    {
      provide: ProviderTransformer,
      useClass: ExchangeCalendarTransformer,
    },
    {
      provide: 'exchangeCalendarProvider',
      useClass: ThirdPartyCalendarService,
    },
  ],
  exports: ['exchangeCalendarProvider'],
)
export class ExchangeCalendarModule {}

@Module({
  imports: [ExchangeCalendarModule, GoogleCalendarModule],
  exports: [ExchangeCalendarModule, GoogleCalendarModule]
})
export class ProvidersModule {}

You can make it even simpler if you don't have to use both exchangeCalendarProvider and googleCalendarProvider at the same time within a single scope (which I believe should be feasible too).

kierans commented 4 years ago

Thanks @jmcdo29 and @kamilmysliwiec for your suggestions. πŸ‘ It's been a good discussion. πŸ˜„ Both suggestions are technically feasible within Nest currently, however as you'll agree there's a bunch of extra wiring needed to accommodate the goal of using useClass.

What this feature request is about, is modifying Nest to allow useClass to use the inject array as I take the view that the wiring for classes shouldn't be in the classes, and thus my preference is to actually use dynamic providers where inject replaces usages of @Inject() without the need for wrapper factory functions. Is that something that the core Nest team is willing to consider/do (or allow PRs for)? If not, then this Feature Request can be closed as it's not going to be implemented and there's a solid discussion trail.

WonderPanda commented 4 years ago

In the absence of something baked directly into the core we can go a long way to removing the factory boilerplate with a plain old function. Here injectClass(Class1) will automatically pass through the values from the inject array to the Class1 constructor.

import { Module } from '@nestjs/common';
import { AppController } from './app.controller';
import { AppService } from './app.service';

class Class1 {
  constructor(private readonly dep1: number, private readonly dep2: string) {}

  doSomething() {
    console.log(`DOING SOMETHING: ${this.dep1}-${this.dep2}`);
  }
}

const injectClass = (ctor: new (...args: any[]) => any) => (...args: any[]) =>
  new ctor(...args);

@Module({
  imports: [],
  controllers: [AppController],
  providers: [
    AppService,
    {
      provide: 'Token1',
      useFactory: () => 42,
    },
    {
      provide: 'Token2',
      useFactory: () => 'aString',
    },
    {
      provide: 'Token3',
      // useFactory: (dep1: number, dep2: string) => new Class1(dep1, dep2),
      useFactory: injectClass(Class1),
      inject: ['Token1', 'Token2'],
    },
    {
      provide: 'Token4',
      useFactory: (class1: Class1) => {
        class1.doSomething();
        return 'boop';
      },
      inject: ['Token3'],
    },
  ],
})
export class AppModule {}
WonderPanda commented 4 years ago

Taking the abstraction further for fun we could just define this:

type Ctor = new (...args: any[]) => any;

const injectClass = (ctor: Ctor) => (...args: any[]) =>
  new ctor(...args);

const autoInjectedClassProvider = (
  provide: FactoryProvider['provide'],
  ctor: Ctor,
  ...inject: FactoryProvider['inject']
): Provider => ({
  provide,
  useFactory: injectClass(ctor),
  inject,
});

And then in the providers array you can just do:

autoInjectedClassProvider('Token3', Class1, 'Token1', 'Token2'),

Which results in an instance of Class1 being provided with the key Token3 and the deps represented by Token1 and Token2 being automatically injected into it's constructor

Or with your original example:

@Module({
  providers: [
    autoInjectedClassProvider('googleCalendarProvider', ThirdPartyCalendarService, GoogleCalendarTransformer),
    autoInjectedClassProvider('exchangeCalendarProvider', ThirdPartyCalendarService, ExchangeCalendarTransformer),
  ],
  exports: [
    "googleCalendarProvider",
    "exchangeCalendarProvider"
  ]
})
class ProvidersModule {

}
kamilmysliwiec commented 4 years ago

Thanks for sharing your ideas @WonderPanda!

We don't plan to add the inject to a ClassProvider interface.