TypeOrmModuleAsyncOptions.useConnection

[mkelandis]

Feature Request

Suggestion: TypeOrmModule.forRoot & TypeOrmModule.forRootAsync would be more extensible if the interface gave the caller a way to implement a function that directly returned a typeorm connection. Exposing that to the calling code would make the feature more extensible.

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

We use postgres 12 in production and sqlite inmem for local e2e tests. Our application generally follows the TypeOrmModule.forRootAsync & TypeOrmModule.forFeature pattern described here: https://docs.nestjs.com/techniques/database

The decision for which db to use is currently defined in the environment variables used by the configservice and app.module.

The team would like to try pg-mem instead of sqlite. There is currently no support for pg-mem in TypeOrmModuleAsyncOptions. pg-mem lets us create a connection like this:

https://github.com/oguimbal/pg-mem/blob/5686ff5eb56ac46eb59b5498e0c22e0b6b6931c6/samples/typeorm/simple.ts

    //==== create a memory db
    const db = newDb({
        // 👉 Recommanded when using Typeorm .synchronize(), which creates foreign keys but not indices !
       autoCreateForeignKeyIndices: true,
   });

    //==== create a Typeorm connection
    const got: Connection = await db.adapters.createTypeormConnection({
        type: 'postgres',
        entities: [User]
    });

^-- There is currently no way to make a connection with typeorm and inject it into the TypeormModule. The only solution would be to refactor all of the code to use: https://docs.nestjs.com/recipes/sql-typeorm which exposes a bunch of boilerplate stuff that we don't really need.

Describe the solution you'd like

We could add a new function to the existing interface that returns an actual typeorm connection as an alternative to TypeOrmModuleAsyncOptions.

The updated interface could look something like:

export interface TypeOrmModuleAsyncOptions extends Pick<ModuleMetadata, 'imports'> {
    name?: string;
    useExisting?: Type<TypeOrmOptionsFactory>;
    useClass?: Type<TypeOrmOptionsFactory>;
    useFactory?: (...args: any[]) => Promise<TypeOrmModuleOptions> | TypeOrmModuleOptions;
    useConnection?: () => Promise<Connection>;
    inject?: any[];
}

Teachability, Documentation, Adoption, Migration Strategy

By adding the new optional function to the interface the code remains backwards compatible. We can document how to implement useConnection in the examples on the site.

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

Give users more control over the typeorm connection used by the TypeOrmModule.

The way I'm reading the code, NestJS needs to keep up with changes in typeorm whenever support for a new database is added or the mechanism to configure and create a connection changes. Implementers of the new function could directly create the connection according to the typeorm doco. This is is a looser coupling with typeorm and means that we can take advantage of typeorm releases without worrying about NestJS mirroring the options in TypeOrmModuleOptions.

[Tony133]

Hi,

interesting feature.
Since it only affects the typeorm module, you should move the typeorm "issues" to the repository of the typeorm nestjs module
--> https://github.com/nestjs/typeorm

[Tony133]

this and the pull --> #718

Let's wait @kamilmysliwiec for approval or not :-)

[kamilmysliwiec]

@mkelandis have you tried using the overrideProvider() method (which is exactly for this purpose in e2e tests)?

Test.createTestingModule({})
   .overrideProvider(Connection)
   .useValue(yourDbConnection)
   .compile();

https://docs.nestjs.com/fundamentals/testing#end-to-end-testing

[mkelandis]

Thank you @kamilmysliwiec - I will poke at that solution and see if it unblocks me.

[mkelandis]

Update - this does unblock me. I now am far enough along to see that pg-mem isn't baked enough yet to support our migrations :) , which rely on foreign keys with actions, in some cases.

example:

        await queryRunner.query(`ALTER TABLE "company_address" ADD CONSTRAINT "FK_2509f69fcfd83d6928072f8bac9" FOREIGN KEY ("companyId") REFERENCES "company"("id") ON DELETE CASCADE ON UPDATE NO ACTION`);

Error Message for those who might get here via a web search:

Not supported: Foreign keys with actions not yet supported

I think the Feature Request might still make sense and have some loose-coupling advantages long-term but I'm also fine with closing this out if folks think that it isn't needed / not priority. Thanks for the attention, and let me know if you want me to close this out.

[Tony133]

Well then I can close the pull I made as it is not needed?

[kamilmysliwiec]

@mkelandis I'm glad it helped :)

@Tony133 yeah let's close the PR then!

[navjotahuja92]

In case someone needs this, I have a working example: https://gist.github.com/navjotahuja92/f51601b17fb248cf4727b5650d945607