-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(common): pass options to nested async modules #9935
feat(common): pass options to nested async modules #9935
Conversation
Pass parent module providers to 'provideInjectionTokensFrom' in ConfigurableModuleAsyncOptions in order to pass options down in nested async modules. Only necessary providers are taken by recursively looking on the 'inject' array.
Pull Request Test Coverage Report for Build b658d291-262f-4540-945f-84bbac15a090
💛 - Coveralls |
This isn't correct.
Can you please share an example (real-world code example) that illustrates the problem? What are you trying to accomplish that isn't currently feasible? |
Sure. Let's say I have this type of application (this is not runnable, just an example): @Module({
imports: [
ConfigModule,
AuthenticationModule.registerAsync({
imports: [ConfigModule],
useFactory(config: ConfigService) {
return config.authentication;
},
inject: [ConfigService],
}),
],
})
class AppModule {}
class AuthenticationModule {
static registerAsync(opt) {
//...
return {
providers: [...this.getAsyncProviders(opt)],
imports: [
// this is not working because AuthenticationOptions' provider is not present in the JwtModule
JwtModule.registerAsync({
useFactory(opt: AuthenticationOptions) {
return opt.jwtOptions;
},
inject: [AuthenticationOptions],
}),
],
};
}
} The reason why I say that all modules should be registered asynchronously is because each module should have its own options, and those options will determine the options of the child modules in a decoupled way: I don't want my AuthenticationModule to know about my ConfigService, that's why I use an async import; same thing for the JwtModule. That is how synchronous modules behave. But since we use a ConfigModule, we are then forced to use async modules down the hierarchy in order to inject the options. This example breaks because the JwtModule does not have the AuthenticationOptions provider in its providers. I cannot add my AuthenticationModule to the imports because that would be a circular dependency. The only solution is to add the AuthenticationOptions provider the JwtModule's providers. But this will also break because the AuthenticationOptions provider needs the ConfigService, which is also not part of the JwtModule. I hope you see what I mean. That's why I want something that resolves all my inject dependencies recursively by passing down all options' providers. |
If your So, just to make sure that I'm on the same page, the solution proposed in this PR tends to solve this issue by exposing an additional property that let you duplicate the parent module's providers within the scope of the child module so they can be used to construct its own configuration. Is that correct? |
That's correct. I'll point out again that only the providers required to construct the child module's options will be provided to the child module. |
I had the same problem and I tried as below. await Test.createTestingModule({
imports: [MongoModule.forRootAsync({
imports: [ConfigModule.forRoot({ ... })],
useFactory: (config: ConfigService) => ({ ...config.get('mongo') }),
inject: [ConfigService],
})],
}).compile();
/* mongo.module-definition.ts */
export const {
ConfigurableModuleClass,
MODULE_OPTIONS_TOKEN,
} = new ConfigurableModuleBuilder<MongoOptions>().setClassMethodName('forRoot').build();
/* mongo-config.service.ts */
@Injectable()
export class MongooseConfigService implements MongooseOptionsFactory {
constructor(@Inject(MODULE_OPTIONS_TOKEN) private mongo: MongoOptions) {
// Logs are output normally when defining services in providers.
console.log('MongooseConfigService', mongo);
}
public createMongooseOptions(): MongooseModuleOptions {
return { ... };
}
}
/* mongo.module.ts */
import { MongooseModule } from '@nestjs/mongoose';
...
@Module({
imports: [
// first try
// Nest can't resolve dependencies of the MongooseConfigService (?). Please make sure that the argument CONFIGURABLE_MODULE_OPTIONS[19a70477-4fdc-45fc-9609-1cafb87b7f85] at index [0] is available in the MongooseCoreModule context.
MongooseModule.forRootAsync({
useClass: MongooseConfigService,
inject: [MODULE_OPTIONS_TOKEN],
}),
// second try
// Nest can't resolve dependencies of the MongooseModuleOptions (?). Please make sure that the argument MongooseConfigService at index [0] is available in the MongooseCoreModule context.
MongooseModule.forRootAsync({
useFactory: (config: MongooseConfigService) => config.createMongooseOptions(),
inject: [MongooseConfigService],
// Nest can't resolve dependencies of the MongooseModuleOptions (?, MongooseConfigService). Please make sure that the argument CONFIGURABLE_MODULE_OPTIONS[31d160a5-c894-4ebf-87f6-bdfc90a8afd2] at index [0] is available in the MongooseCoreModule context.
// inject: [MODULE_OPTIONS_TOKEN, MongooseConfigService],
}),
],
providers: [MongooseConfigService],
})
export class MongoModule extends ConfigurableModuleClass {} |
LGTM |
Pass parent module providers to 'provideInjectionTokensFrom' in ConfigurableModuleAsyncOptions in order to pass options down in nested async modules.
Only necessary providers are taken by recursively looking on the 'inject' array.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Right now it is not easy to have a hierarchy of async modules because options' providers are not passed down the hierarchy. This is an example of the problem. The solution I gave (pushing the options' providers after creating the module) breaks as soon you have more than one level of nesting.
What is the new behavior?
In an ideal application all modules should be imported asynchronously and each options' provider should be passed down the hierarchy of modules in order to resolve every
inject
dependency.So I added an optional
provideInjectionTokensFrom
to the interfaceConfigurableModuleAsyncOptions
, which takes the parent module's providers, and only injects in the child module those providers that are recursively required by the child moduleinject
array (usually the parent options' provider).Right now you can implement my new solution with the
extras
feature, but since this use case is basically universal I think it should be part of the core feature.Now you can define a hierarchy of modules like this:
The only problem is when options' tokens have the same name string down the path. This can be avoided by using symbols as tokens.
Does this PR introduce a breaking change?
Other information