batch-execute enhancements #3588
Conversation
🦋 Changeset detectedLatest commit: 7229033 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/theguild/graphql-tools/8i7BBqoyptioGuce3ceA68k5PND5 |
gmac
changed the title
|
It all looks great to me. Very important to highlight that batch execution as implemented here is based almost entirely on the amazing work at Gatsby by @vladar see gatsbyjs/gatsby#22347 |
|
I added that Gatsby PR link into the documentation page to give due credit. Rah rah, open source! |
…to gmac/batch-execute
|
Calling this installment ready to go, @ardatan. I'll have a followup to this that fixes some bugs with fragments... those will probably be best reviewed on their own. |
|
|
||
| if (executionVariableNames.length > 0) { | ||
| if (executionVariableNames.length > 0 || hasFragmentDefinitions) { |
There was a problem hiding this comment.
@yaacovCR – have a look at 44ba4cb. I've fixed a couple bugs related to fragments here:
- Fragments weren't getting prefixed unless there were also variables.
- Inlined root fragments were allowed to leave an invalid orphaned fragment definition. This now filters out fragment definitions unless they have a surviving spread implementation.
|
Thanks for the feedback, gents. Looks tighter now and fixes more bugs. |
| import { DocumentNode, DefinitionNode, OperationTypeNode, OperationDefinitionNode, Kind } from 'graphql'; | ||
|
|
||
| export function operationTypeFromDocument(document: DocumentNode): OperationTypeNode { | ||
| for (const def of document.definitions) { |
There was a problem hiding this comment.
Maybe getOperationAST from graphql-js. But in case of multiple operations given in the same document, we will need operationName as well.
|
Thank you @gmac <3 |
Description
Batch execution is a really good feature. Unfortunately, it's a hidden gem within the mono-repo at the moment with little attention given outside of the built-in schema stitching option. This PR works on changing that by making batch execution friendlier for general use. There are some possible points of contention here, so let's discuss...
BUG FIX: See 44ba4cb. The current implementation has a couple bugs involving fragment definitions:
Within
splitResults, errors without a path are now returned to all requests in the batch. This is really important for general usability: right now, batch execution swallows unpathed errors such as schema validations and complexity limits. These errors (particularly schema validations) will rarely be encountered by schema stitching, but are commonplace when batching hand-written queries. The downside of returning all unpathed errors is that all results in the batch will report the same validation error even though it may only apply to one request in the batch, but over-reporting feels better than under-reporting.I've reduced the
graphqlTools0_prefix down to just_0_... heavily inspired by Bramble. Given that batching rewrites all root scope aliases, we're not really at risk of conflicting with anything, and thegraphqlToolsprefix divulges too much information about the underlying implementation, IMO. The simpler prefix is more generic and doesn't encourage analytics or instrumentation based on weak signals. It also lends itself to being documented as a generic capability in higher-level product features.Adds a local test suite for the batch-execute package.
Lastly, I've added a page of documentation about this fantastic package that is both remarkably powerful and quite easy to use. With documentation circulating, we may see some renewed interest in this hidden gem of GraphQL Tools.
Curious of your thoughts here, @yaacovCR.
Type of change
Please delete options that are not relevant.
Checklist:
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...