Skip to content
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

Understanding the plugin use cases #7

Open
yossisp opened this issue Mar 7, 2021 · 2 comments
Open

Understanding the plugin use cases #7

yossisp opened this issue Mar 7, 2021 · 2 comments

Comments

@yossisp
Copy link

yossisp commented Mar 7, 2021

After going through the code of the plugin it seems that the plugin only works with the default resolvers provided with graphql-compose (e.g. findById or findByIds). Am I correct? So if I'm using a custom resolver the plugin will not help me. I think it will be nice to note this in the readme because users might expect that the plugin automatically provides dataloader functionality for all resolvers (at least this is what I expected).

In addition, on this line for findByIds loader I'm wondering why findByIdResolver.resolve is called in resolve(resolveParamsArray.map(rp => findByIdResolver.resolve(rp))). Shouldn't findByIdsResolver be given an array of id's and called once? Same goes for this line shouldn't findManyResolver.resolve be given an array of id's and resolve them in one call? Without this modification as far as I understand the only advantage of using the plugin is that it batches calls to resolvers. But it doesn't group database queries. Am I correct?

@nodkz
Copy link
Member

nodkz commented Mar 9, 2021

Yep, you are correct. There is a lot of things which can be improved. For example, if findByIdsResolver exists then use it instead of the map with findByIdResolver. How I understand Stoffern made this dataLoader to reduce the number of requests to the database when you need to load the same records several times (eg you request 100 articles with 5 authors, so without this dataLoader you request authors 100 times, with this dataLoader just 5).

If you want to send Pull Request to this plugin feel free to do it. Frankly, I don't use this package in my projects, so it will be nice if somebody takes care of this plugin. 😉

@yossisp
Copy link
Author

yossisp commented Mar 21, 2021

Because graphql-compose-mongoose already has a built-in dataloader I don't think it will be useful to provide the same functionality in this package. However, if you think it would be helpful I can write a code example which uses a custom dataloader with graphql-compose-mongoose in graphql-compose-mongoose repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants