New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How to use DataLoader? #1135
Comments
You probably wanna start with Interceptors. |
@twilroad Nope, but link above is basically everything you need to get started. Interceptor can be used for caching (you can see it in docs), but with slight modifications you can store result from next handler. As a param, you will need a cache key, you can pass it like shown here https://docs.nestjs.com/advanced/mixins. |
Sample Code
|
How would this work with the new GraphQLModule.forRoot({...options})? Where should we initialize the context? Maybe creating a wrapper module and doing it on configure(consumer) somehow? It should be a different instance of data loader(s) for each different request. |
take a look at this approach |
@kamilmysliwiec Dataloader is supposed to batch all the resolved entity ids and query them in one go as a single SQL query. But for some reason when used in Nest it executes more than one query for a single request. Here is an example Suppose we have a following GQL query query {
persons {
name
address {
street
}
}
} So addresses for all the persons from this query should be fetched using a single SQL query (using dataloader approach) but in reality, I have something like this:
I know that dataloader should be invoked on a Any help is appreciated, maybe I just missed something regarding the dataloader.. I think this is related graphql/dataloader#150 |
For those who faced the same issue, I've mentioned above. I've added a flag that checks whether a user is already authenticated in the guard in order to prevent its redundant executions. @kamilmysliwiec Is this expected behavior that guards (probably interceptors, filters, etc. as well) get executed on every GQL property resolution? |
@rychkog this is fixed in |
@kamilmysliwiec maybe this could be somewhat optional? you disabled all guards, filters and interceptors. what if I want to run guard againts some property? In my case I was running interceptor altering the context and this was breaking change. I think this ability could be actually in ResolveProperty options or something, especially guards should be allowed. |
@trubit it would have a big impact on the performance. I'd suggest moving this logic inside the resolve property handler. |
I agree with @trubit , it's a breaking change. For example, I use guard to control access to my properties, with a AuthGuard and RoleGuard. So now, we will have 2 different implementations to check roles and auth ... |
I imagine this line |
@zuohuadong How I do it is I use request-scoped providers that create instances of injectable services that have dataloaders in them.
Then you simply inject it as a regular service in the resolver.
This way you can also use the dataloaders anywhere in the code, like in service classes, not only in resolver methods. |
@vnenkpet this throws an error Nest can't resolve dependencies of the UserDataLoader (?). |
it took me like 2 days debugging how to use DataLoader with Nestjs, and finally I could make it. first create your contract as @vnenkpet did.
then create your user loader for example.
In my scenario, I had to use this userService at another module, so here is the code for that. at your users module you must put that service at providers property in order to work, and you have to export it from there as you are going to use it in another module ( so you can import it from there )
at the other module you are trying to use the loader at ( which in my case was products module and I needed to fetch users associated with these products.)
then finally at ProductResolver, I injected it.
then when resolving the property at the same resolver.
|
I came up with the following that seems to work nicely.
@Module({
imports: [TypeOrmModule.forFeature([FixtureRepository])],
providers: [FixtureService, FixtureLoaders, FixtureResolvers],
})
export class FixtureModule {}
@Injectable({ scope: Scope.REQUEST })
export class FixtureLoaders {
constructor(private readonly fixtureService: FixtureService) {}
public readonly findByCode = new DataLoader<number, Fixture>(async codes => {
try {
const fixtures = await this.fixtureService.findByCodes(codes)
return codes.map(code => fixtures.find(fixture => fixture.code === code))
} catch (error) {
throw error
}
})
}
@Injectable()
export class FixtureResolvers {
constructor(private readonly fixtureLoaders: FixtureLoaders) {}
@Query('getFixtureByCode')
async getFixtureByCode(
@Args('code') code: number,
): Promise<FixtureRO | undefined> {
try {
const fixture = await this.fixtureLoaders.findByCode.load(code)
if (fixture !== undefined) {
return fixture.toResponseObject()
} else {
return undefined
}
} catch (error) {
throw error
}
}
} |
@secmohammed @jeppe-smith For some reason, your solution doesn't fix the n+1 problem, I've enabled the query logger in mysql and I can see that the fetch queries for the related objects are fired one by one. |
Dataloader is designed to end up the cached version by the end of the request. It was designed to do so. this will help you to batch a multiple requests with the ids and by the end of the promise fetch all of the requests at once. you can integrate with a Redis server or so to accomplish having a cached version of your data. Also, Having a N+1 is basically based on your sql query you are trying to do, it should be where id is in (...your ids), and this one will be batched at the very last of your promise to fetch them at once. However, I don't know how Scope.Request makes the change. |
The problem seems be that the DataLoader is instantiated multiple times inside the request: on each result... |
I agree with @pablor21, I encountered the same problem with multiple instance by request. May be it's a bug with Nest. |
@pablor21 How are you sending requests that cause multiple instances of the dataloader? If I send a query like query {
first: getFixtureByCode(code: 855172) {
id
}
second: getFixtureByCode(code: 855172) {
id
}
third: getFixtureByCode(code: 855174) {
id
}
} I get a single instance of dataloader and my database is queried once with |
I created a small npm package, nestjs-dataloader, since I also struggled with this. Hopefully it helps someone else. |
I struggled a lot trying to understand this, I'll just write my solution which probably isn't the cleanest but it's easiest for myself at least PS: the "genericOneToManyDataLoader" can be of course custom made made for every field, I was just experimenting some generic way (btw, if someone can help me make it typesafe I would appreciate a lot, I don't know how to do it, now it just returns "any[]") export const genericOneToManyDataLoader = (t: any, keyField: string) =>
new DataLoader<number, typeof t[]>(async (keys: number[]) => {
const items = await getRepository(t).find({
where: { [keyField]: In(keys) },
});
return keys.map(id => items.filter(el => el[keyField] === id));
});
export interface MyContext {
req: Request;
res: Response;
commentDataLoader: ReturnType<typeof genericOneToManyDataLoader>;
postDataLoader: ReturnType<typeof genericOneToManyDataLoader>;
}
@Module({
imports: [
PostModule,
CommentModule,
UserModule,
GraphQLModule.forRoot({
playground: true,
debug: true,
autoSchemaFile: true,
installSubscriptionHandlers: true,
context: ({ req, res }) => ({
req,
res,
commentDataLoader: genericOneToManyDataLoader(Comment, 'postId'),
postDataLoader: genericOneToManyDataLoader(Post, 'userId'),
}),
}),
DatabaseModule,
],
controllers: [AppController],
providers: [AppService],
})
export class AppModule {} // user.resolver.ts -- its literally the same in comments
@ResolveField(() => [Post], { nullable: 'items' })
async posts(@Parent() parent: User, @Context() ctx: MyContext) {
return ctx.postDataLoader.load(parent.id);
} |
UPDATE: I needed services in loaders. and the above solution of mine was bad, I ended up using @jeppe-smith s one which worked very well Thank you a lot for that, I don't understand why others say this generates multiple dataloaders/queries, I get only one per request as expected
|
This is what I've got: import { Injectable, Scope } from '@nestjs/common'
import { InjectRepository } from '@nestjs/typeorm'
import DataLoader from 'dataloader'
import { Repository } from 'typeorm'
import { Project } from './project.entity'
@Injectable({ scope: Scope.REQUEST })
export class ProjectLoader {
constructor(
@InjectRepository(Project)
private readonly projectRepository: Repository<Project>,
) {}
readonly findOneByIds = new DataLoader<string, Project | undefined>(async (ids) => {
const projects = await this.projectRepository
.createQueryBuilder('project')
.where('project.id IN (:...ids)', { ids })
.getMany()
return ids.map((id) => projects.find((n) => n.id === id))
})
} Perhaps it's a naive attempt, but I've made the Anyone else having this issue? EDIT: import { Inject, Injectable, Scope } from '@nestjs/common'
import { CONTEXT } from '@nestjs/graphql'
import { InjectRepository } from '@nestjs/typeorm'
import DataLoader from 'dataloader'
import { Repository } from 'typeorm'
import { Project } from './project.entity'
interface RequestUser {
req: {
user: { id: string }
}
}
@Injectable({ scope: Scope.REQUEST })
export class ProjectLoader {
constructor(
@Inject(CONTEXT) private context: RequestUser,
@InjectRepository(Project)
private readonly projectRepository: Repository<Project>
) {}
readonly findOneById = new DataLoader<string, Project | undefined>(
async (ids) => {
const user = this.context.req.user // Would ideally use decorator for this instead (i.e. from createParamDecorator)
const projects = await this.projectRepository
.createQueryBuilder('project')
.where('project.id IN (:...ids)', { ids })
.andWhere('project.owner = :userId', { userId: user.id })
.getMany()
return ids.map((id) => projects.find((n) => n.id === id))
}
)
} |
This is by far the most elegant solution. |
I too have the problem when implementing the soltion marked by @kamilmysliwiec as Any ideas? |
@Artimunor I was having a problem with @incompletude solution because not every field resolver in my application was using dataloaders, thus desyncing the execution and preventing the dataloaders to batch. When I applied the dataloaders to every field resolver of my graph it worked correcty batching every query on the same depth of the graph. |
Thanks for this man! I've been looking on this solution, works perfectly on @ResolveField. |
For some reason, the |
Hey guys. I also tried the solution provided by @jeppe-smith. This works fine on ResolveFields and such, but only on Queries. Could be my general misunderstanding of the whole graphQL part of NestJS but I stumbled upon it while implementing this solution. I had as well problems to implement the callBack async like described here. Just to mention that for others: The dataloaders on the ResolveFields are not getting used in case of a mutation or subscriptions. |
This solution first seems elegant, but it quite a bad design because the dataloader is a request scope provider, so any other providers that depend on it will also become request scope. It means that the resolver provider will be recreated every incoming request. Refer to https://docs.nestjs.com/fundamentals/injection-scopes#scope-hierarchy |
Just FYI: there's nothing wrong with that. This is precisely why the "request-scoped" providers feature was added to the framework. |
Haven't seen dataloaders for typeorm relationships like https://github.com/slaypni/type-graphql-dataloader#with-typeorm Has anyone done anything like that? I'm assuming the above doesn't work with nestjs/graphql since it doesn't use type-graphql. |
I strongly believe this should be properly addressed in docs. At least some guidance regarding how to use |
For anyone who is struggling with this, I wrote an article about How I integrated Dataloader with NestJS Hope this helps anyone else who is struggling with the same problem 😃 |
Are there any relevant examples?
https://github.com/facebook/dataloader
The text was updated successfully, but these errors were encountered: