Request-scoped transaction is better served by NestJS Injection Scoping

[devin-fee-ah]

nestjs/src/mikro-orm.middleware.ts

Lines 4 to 13 in d24dd06

	@Injectable()
	export class MikroOrmMiddleware implements NestMiddleware {
	constructor(private readonly orm: MikroORM) {}
	use(req: unknown, res: unknown, next: (...args: any[]) => void) {
	RequestContext.create(this.orm.em, next);
	}
	}

It seems like this would be better served using a request-based injection-scope as described here (https://docs.nestjs.com/fundamentals/injection-scopes) and using that for the middleware.

I can prototype something up, too.

[B4nan]

Not really, there were some issues with that, iirc it was something with JWT middleware not supporting the nest di request scope.

But we could have that available optionally, agreed it would be cleaner. Btw it is now possible to disable the automatic request scope middleware registration.

[jsprw]

I have been using a modified provider of the EnitityManager token and it's working fine for me (disabled the request context option):

export const createMikroOrmEntityManagerProvider = (
  alias?: string
): Provider => ({
  provide: alias ?? EntityManager,
  scope: Scope.REQUEST,
  useFactory: (orm: MikroORM) => orm.em.fork(),
  inject: [MikroORM],
});

I could create a PR with this change for you to test it (altough it's an easy fix)?

[B4nan]

The thing is that the native request scope from nest does not work with things like their JWT service. This is the issue I am talking about:

nestjs/nest#1870

This is for me the main reason why to prefer the RequestScope solution (that can be nowadays also replaced with AsyncLocalStorage), as it just works. Also it is arguable that using the nest DI is "better". It will instantiate all the classes for you for each request, which will have a performance impact. And it is not just about the EM or repositories - this applies to all classes that are dependent on anything ORM related, so most probably all of your services and controllers. The domain api solution also have impact, but when replaced with the AsyncLocalStorage, there was almost no overhead.

So again, I am fine with supporting this optionally, feel free to send a PR, but it needs to be configurable, and I would keep the RequestContext solution as the default.

[jsprw]

See my comment here why I prefer using this over RequestContext: mikro-orm/mikro-orm#315 (comment). Angular's zone.js and with this approach of getting the repositories you can't really use the Zone'js solution that easy either.

Alright, will create a PR in a bit.