search

krislefeber / nestjs-dataloader

Dataloader plugin for NestJS
https://krislefeber.github.io/nestjs-dataloader/
MIT License
148 stars 45 forks source link

Fix context Id and make loader work with request scope #14

Closed tarazou9 closed 4 years ago

tarazou9 commented 4 years ago

Fix context Id and make loader work with request scope

tarazou9 commented 4 years ago

@krislefeber Would you please take a look at this and let me know? :)

krislefeber commented 4 years ago

@tarazou9 , I'm not sure I understand what feature your changes provide. It appears to be functionally equivalent to how it was before your changes. Am I missing something? I see that you have the context id, but it's never being used. In fact, the previous code was already using the request object to tie the dataloader object to, so it was in fact request scoped

lennykean commented 4 years ago

@krislefeber The data loader is forced into singleton scope from a DI perspective because of the use of ModuleRef.get. While it does get attached to the request, it must first be resolved, and with the current implementation, that can't happen unless the dataloader and its DI subtree is all singleton scope.

For example, if your NestDataLoader<> service has a dependency Foo that is request scoped, Foo will fail to resolve because the call to get can't locate a request scoped service. @tarazou9's fix is switching it to a resolve. That also means DataLoaderInterceptor must create and store a contextId.