Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

typeorm-sql-js-example

npm i https://pkg.pr.new/typeorm/typeorm@11701

commit: ee1b60d

coverage: 76.418% (+0.006%) from 76.412%
when pulling ee1b60d on alumni:feat/query-runner-async-dispose
into ea7a99a on typeorm:next.

Thanks for this lovely feature @alumni

@gioboa What do you think about the transactions that are in progress in that QueryRunner that is being disposed?

Right now, we only call QueryRunner.release(), i.e. return the client connection to the pool, at least in most drivers. Not all have a pool: SQLite just flushes the data to the disk.

Not all drivers support a transaction (e.g. MongoDB, some of the SQLite drivers), but for those that do, we could have a transaction in progress. So I was wondering, should we commit the transaction? Should we roll it back if not explicitly committed?

Also, some databases have save points ("nested transactions", e.g. Postgres) - how should those be handled? Should they all be committed?

I'm thinking to add:

async [Symbol.asyncDispose](): Promise<void> {
    try {
        while (this.isTransactionActive) {
            await this.commitTransaction();
        }
    } finally {
        await this.release()
    }
}

That's a hard decision, but I think your suggestion is a good one. We can probably have a voting session with the maintainer team to agree with this.

Okay, I think I figured out that we don't need to release all savepoints, we can simply commit once. Should work in most drivers based on my understanding. Pushing a commit now.