Nest: Allow async NestModule.configure method

[michaelbromley]

I'm submitting a...

[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Currently the NestModule interface has a confugure() method which is executed in a synchronous manner:

https://github.com/nestjs/nest/blob/ff2e310a188ef912c1c79e0fcc5611680c8e8dba/packages/common/interfaces/modules/nest-module.interface.ts#L3-L5

Desired behavior

I have some async logic which I need to run inside the configure() method, but this is not currently supported. As far as I can see it would be a minor and localized change which would allow this:

// nest-module.interface.ts
export interface NestModule {
  configure(consumer: MiddlewareConsumer): MiddlewareConsumer | void | Promise<MiddlewareConsumer | void>;
}

// middleware-module.ts
 public async resolveMiddleware(
    middlewareContainer: MiddlewareContainer,
    modules: Map<string, Module>,
  ) {
    await Promise.all(
      [...modules.entries()].map(async ([name, module]) => {
        const instance = module.instance;

        await this.loadConfiguration(middlewareContainer, instance, name);   // < --- here
        await this.resolver.resolveInstances(module, name);
      }),
    );
  }

public async loadConfiguration(                                              // < --- here
    middlewareContainer: MiddlewareContainer,
    instance: NestModule,
    module: string,
  ) {
    if (!instance.configure) return;

    const middlewareBuilder = new MiddlewareBuilder(this.routesMapper);
    await instance.configure(middlewareBuilder);                             // < --- here

    if (!(middlewareBuilder instanceof MiddlewareBuilder)) return;

    const config = middlewareBuilder.build();
    middlewareContainer.addConfig(config, module);
  }

plus one test in middlewares-module.spec.ts

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

In my particular case I have a dynamic import occurring during the execution on configure() which returns a promise, making the whole thing async.

Environment

Nest version: 5.4.1

[kamilmysliwiec]

In my particular case I have a dynamic import occurring during the execution on configure() which returns a promise, making the whole thing async.

Hi @michaelbromley, could you give more details about your particular use case? Perhaps, there is another way that could be potentially considered instead.

[michaelbromley]

In my app I need to run a function which requires the TypeORM Connection object. The reason is that I allow the user to configure custom fields on the TypeORM entities via a config object, and I want to run some validation on that config (ensure no naming conflicts etc) before TypeORM attempts to create the table structure.

I use the TypeORM getConnection() function to get a reference to the connection, so this needs to be run after the TypeOrmModule.forRoot() method has run, i.e. once the Nest app has started bootstrapping, otherwise the connection has not yet been created.

My intention was to be able to throw an error on a validation failure before TypeORM tries to create the table structure. I thought the configure() method was run early enough for this, but on further investigation it appears not.

So it might be that I am just abusing this method, in which case this issue can be closed.

[kamilmysliwiec]

So it might be that I am just abusing this method, in which case this issue can be closed.

Got it. Btw, you can inject Connection through class constructor (instead of using getConnection() function) :)

[zailic]

I was just preparing to create an identical issue with this... @kamilmysliwiec is there any particular reason why you don't want configure() method to be asynchronous? I have a similar issue:

export class BotModule {
    private logger: Logger;
    constructor(
        private readonly botService: BotService,
        private readonly configService: ConfigService
    ) {
        this.logger = new Logger(BotModule.name);
    }
    async configure(consumer: MiddlewareConsumer): Promise<void> {
        this.logger.log("Configuring module", BotModule.name);
        await this.botService.init(); // asyncronous stuff - e.g. authenticating with third party services
        consumer.apply(
            this.botService.getConnector().listen() // because of the "await" above the middleware will not be registered by nest runtime
        ).forRoutes({
            path: `${BASE_BOT_URI}`,
            method: RequestMethod.POST,
        });
        this.logger.log('Configuration done');
    }
}

A live example that simulates the issue above:
Hello world async sample

[dotdevru]

+1

[kamilmysliwiec]

Added in 6.6.0 :)

[johnbiundo]

@zailic From a docs perspective, is the net impact of this change that you can now use async/await on module.configure()?

[zailic]

@johnbiundo If you think in terms of a breaking change, the answer is no. Although I think it would be worth it to be mentioned on Middleware Consumer - docs section

[johnbiundo]

@zailic to rephrase my question 😄

There's no breaking change, but now, optionally, you can use async/await on this method. Is that correct? If so, I can add that to the docs section.

[kamilmysliwiec]

It's true @johnbiundo. I think we could just add one additional line that says async/await is supported as well. Nothing more should be required though.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.