Helper + docs for selectionSet arguments

[gmac]

This is a followup to the work done on selectionSet arguments discussed with @yaacovCR in #1709 (comment).

This PR includes a new helper method in the stitch package called forwardArgsToSelectionSet that builds a gateway-to-subservice arguments passthrough. Implemented, it looks like this:

import { forwardArgsToSelectionSet } from '@graphql-tools/utils';

export default {
  User: {
    chirps: {
      selectionSet: forwardArgsToSelectionSet('{ chirpIds }'),
      resolve(user, args, context, info) {
        return delegateToSchema({
          schema: chirpSchema,
          operation: 'query',
          fieldName: 'chirpsById',
          args: { ids: user.chirpIds },
          context,
          info,
        });
      },
    },
  }
};

By default, forwardArgsToSelectionSet will passthrough all arguments from the gateway field to all root fields in the selection set. For advanced selection sets that request multiple fields, an additional mapping of selection names and their respective arguments may be provided:

forwardArgsToSelectionSet('{ id chirpIds }', { chirpIds: ['since'] })

This PR also contains full docs on how gateway-to-subservice argument passthroughs work, which has been a notable missing detail to date.

TODO:

• If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

[gmac]

@yaacovCR – I'm not super familiar with typescript. Can you shed some light on what I'm doing wrong with the casting of SelectionSetNode that is producing this error...? (code review suggestions welcome) thanks!

##[error]packages/utils/src/selectionSets.ts(43,10): error TS2739: Type '(field: FieldNode) => SelectionSetNode' is missing the following properties from type 'SelectionSetNode': kind, selections

[yaacovCR]

It looks like you are making a deep copy of a selection set by calling JSON stringify and parse, better might be to create a new selection set node, I use the types included within graphql-js available via my ide to figure out how to make the node I need...

[gmac]

@yaacovCR – looks like my typescript typing was way off the mark. This has been cleaned up and tests are passing; object copies no longer rely on serialization.

[yaacovCR]

Looks great. Thanks!

[yaacovCR]

Although this fits criteria for inclusion within utils as imports only graphql, seems like it is only relevant for stitching...

Might it belong within the stitch package?

I am also trying to think of a (maybe) better name for the function returning a function....

[gmac]

Sure, any name suggestions? Living in the stitching package makes sense.

…

On Mon, Jul 20, 2020 at 10:39 PM Yaacov Rydzinski ***@***.***> wrote: Although this fits criteria for inclusion within utils as imports only graphql, seems like it is only relevant for stitching... Might it belong within the stitch package? I am also trying to think of a (maybe) better name for the function returning a function.... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1802 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFRROADVAO3YJJPNKG63MLR4T5WJANCNFSM4PBZWIPA> .

[yaacovCR]

what about something like withGatewayFieldArguments?

Then the whole line reads selectionSet: withGatewayFieldArguments('{ theRequiredField }')

Or maybe selectionSet: passFieldArgsTo('{ theRequiredField }')

Or maybe selectionSet: forwardArgsTo('{ theRequiredField }')

[gmac]

I went with forwardArgsToSelectionSet – it's meaningful, and mentioning "SelectionSet" keeps the intent of the method obvious outside of just its implementation context. Moved into @graphql-tools/stitch package along with supporting tests.

I'm pretty excited about this dynamic selection set capability – it opens the door for all kinds of powerful and optimized request patterns. I've got a cross-service interface implementation that this will really optimize.

[yaacovCR]

These changes are great! And I agree, exciting!

I do wonder in general whether type merging might be a better overall pattern for you.

Also, if you are documentation inclined, new batch delegation option can be useful when stitching or within type merging, you may find that useful as well, but it definitely needs documentation...

[gmac]

sorry, just fixed a name typo in docs... that's it, I'm done here. Ready for merge whenever you are.

[gmac]

Also, if you are documentation inclined, new batch delegation option can be useful when stitching or within type merging, you may find that useful as well, but it definitely needs documentation...

I understand the intent of the "batching" feature – it actually sounds like something we've been doing with our own internal pattern for a while that wraps all delegateToSchema calls in a dataloader scoped by field path. I looked quickly at the PR, but I couldn't picture how it was used. If you'd draft a quick code sample demonstrating its usage, I'd be happy to write a guide (I do like writing docs). Feel free to start a discussion and ping me on it.

[theguild-bot]

The latest changes of this PR are available as alpha in npm: 6.0.15-alpha-ad349394.0

Quickly update your package.json by running:

npx match-version @graphql-tools 6.0.15-alpha-ad349394.0

[yaacovCR]

@gmac sounds pretty similar, except the dataloaders are stored in a weakmap keyed by info.fieldnodes, which is unique per list, ie field path...

For some reason, tests for batch delegate are not, um, using batch delegate, https://github.com/ardatan/graphql-tools/blob/master/packages/batch-delegate/tests/typeMerging.example.test.ts
And instead mirror the non batched type merging example....

This should be fixed!

[yaacovCR]

Oh, I remember -- the initial version of those tests explicitly used batch delegation, but the final version just uses the key field within the merged type configuration which causes the gateway schema to automatically use batch delegation:

https://github.com/ardatan/graphql-tools/blob/master/packages/stitch/src/stitchingInfo.ts#L109-L129

Anyways, this feature needs docs. :)

[gmac]

@yaacovCR – looks like the website still needs a rebuild/deploy.

[yaacovCR]

@ardatan

[ardatan]

Fyi, Adding [deploy_website] prefix to the commit message triggers website deployment. I am doing it now :)

[yaacovCR]

I understand the intent of the "batching" feature – it actually sounds like something we've been doing with our own internal pattern for a while that wraps all delegateToSchema calls in a dataloader scoped by field path. I looked quickly at the PR, but I couldn't picture how it was used. If you'd draft a quick code sample demonstrating its usage, I'd be happy to write a guide (I do like writing docs). Feel free to start a discussion and ping me on it.

https://www.graphql-tools.com/docs/schema-stitching#batch-delegation

[gmac]

Okay, that is exactly what I was imagining; nice to see this becoming a first-class feature.