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

clarify GraphQL::Batch.batch documentation #117

Open
stevecrozz opened this issue Dec 13, 2019 · 4 comments
Open

clarify GraphQL::Batch.batch documentation #117

stevecrozz opened this issue Dec 13, 2019 · 4 comments

Comments

@stevecrozz
Copy link

I want to use GraphQL::Batch to load arguments for running mutations and it's not clear to me from reading the documentation if it is safe for me to use GraphQL::Batch.batch wherever I feel the need in my application. The note in the documentation refers only to using this in unit tests.

Can you provide clarification on whether GraphQL::Batch.batch can be safely used outside the test environment?

@dylanahsmith
Copy link
Contributor

The documentation is lacking in the project, it mostly just has overview documentation in the form of a README, but could use reference documentation.

Outside of a GraphQL context, GraphQL::Batch.batch is needed to set GraphQL::Batch::Executor.current which is used to group loads and to re-use existing load results. Inside of a GraphQL context, like in a mutation, this GraphQL::Batch::Executor.current has already been set, so GraphQL::Batch.batch isn't needed for this purpose and will simply not change GraphQL::Batch::Executor.current.

The other thing GraphQL::Batch.batch does is that it calls ::Promise.sync on the result of the block to force the promise to be loaded and return its result. If this is all you need GraphQL::Batch.batch for, then you could just use Promise.sync directly or even call sync directly on the result if it is always a promise.

@cheerfulstoic
Copy link

One thing that took me a little while to figure out: If you're calling for().load() multiple times in a block (or even setting GraphQL::Batch::Executor.current manually) in order to test that case, you should create all of your promises first and then start calling sync on them. When I call sync immediately on each to get the result it doesn't batch the perform calls

@jramiresbrito
Copy link

Hey guys, thanks a lot for the amazing work in this gem.

Understanding why the loader wasn't working outside a GraphQL Context in our application took a while.
Would be great if you guys could update the docs. I can help, just let me know how.

This gem helped to fix a nightmare N+1 we had in our app, so I know how valuable it is for the Rails Community!

@swalkinshaw
Copy link
Contributor

I can help, just let me know how.

We (at Shopify) don't actually use this library outside of GraphQL so if you want to contribute a section to the README that would be appreciated.

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

No branches or pull requests

5 participants