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

gatsby-source-graphql-multiple - fork, feature or estension? #23552

Closed
ariadne-github opened this issue Apr 28, 2020 · 12 comments · Fixed by #25048
Closed

gatsby-source-graphql-multiple - fork, feature or estension? #23552

ariadne-github opened this issue Apr 28, 2020 · 12 comments · Fixed by #25048
Assignees
Labels
help wanted Issue with a clear description that the community can help with. topic: GraphQL Related to Gatsby's GraphQL layer type: question or discussion Issue discussing or asking a question about Gatsby

Comments

@ariadne-github
Copy link
Contributor

I need a feature currently unsupported by gatsby-source-graphql. It was implemented in a plugin:https://github.com/xebiastudio/gatsby-source-graphql-multiple, with includes also a list of use cases.
In a nutshell, this plugin enables to query different graphql endpoints that share a common schema, and then merge the results under a parameter that dispatches on the different endpoints. So for example, the same schema for different languages that is exposed under different endpoints can be queried from the same schema by tuning a language parameter.
In my particular use case, the endpoints are the same, but a different header should be passed, so it's basically the same problem. That is not possible with the official plugin and schema customization, since the plugin can't be used more than once with the same typeName, and also it specializes the subschema, so two different typenames would be incompatible from the start.

The problem is, this plugin looks abandoned, and also the code is 90% gatsby-source-graphql, basically a fork, with little additions.
This is problematic, because in my use case I also need some features of gatsby-source-graphql that were introduced after this plugin was implemented, so I tried to develop a local version starting from a recent gatsby-source-graphql, and adding the other plugin feature, and that worked perfectly.

But then query batching came out (#22347), and this obviously plays really well with this plugin, since it's often making multiple parallel calls under the hood, so I started again from the latest version and integrated my feature.
Now coming to my question, what to do next? I could keep this as a local plugin or publish it for everyone to use, but it will be soon missing new gatsby-source-graphql features that could be integrated easily, and I'd have to maintain it, but since this plugin is basically an extension of the official one, I feel like it's not meant to be a separate plugin.

Will you consider a feature request, or maybe a pull request (assuming I'll make the feature optional) to integrate this into the official plugin?
The other alternative I can think of would be to expose some hook option in the official plugin to allow me (or anyone else) to integrate this feature as a project one, possibly allowing me to keep new updates, avoiding me or anyone else to have to maintain this feature as a separate plugin, but this seems too complicated to me.

What do you think about it?
Can anyone suggest other solutions?

@ariadne-github ariadne-github added the type: question or discussion Issue discussing or asking a question about Gatsby label Apr 28, 2020
@LekoArts LekoArts added the topic: GraphQL Related to Gatsby's GraphQL layer label Apr 28, 2020
@vladar vladar self-assigned this Apr 28, 2020
@vladar
Copy link
Contributor

vladar commented Apr 28, 2020

Hey @ariadne-github !

Thanks for opening this! We really appreciate you taking the time to open it.

Your use-case sounds interesting although a bit exotic. I don't think it is feasible to support all of the use-cases but we could consider adding more hooks and options to make it possible.

Yet the first step would be to try to imagine how something like this could be implemented with existing tools.

We expose createSchema option. So, in theory, you could merge schemas and pass in a final merged schema (probably not even needed if your schemas are identical).

Then querying against this schema is implemented via apollo link abstraction which is composable. We expose another option createLink which allows you to control how fetching is implemented.

So technically you can just write your own link which would send requests to different endpoints depending on the query.

Did you try doing something like this? Are there any missing pieces why this didn't work for you?

@ariadne-github
Copy link
Contributor Author

Hi @vladar,
thank you for your quick response.
I know you cannot support everything, and I know that my use case is a bit exotic, but that already existing plugin made me think that it may not be so uncommon.

I already use the createSchema option to patch the original schema for other reasons (and that's because the other plugin didn't work for me, since that option was more recent).
In reality, I think the schema is the smallest problem here. I'll explain better:

The schema created by gatsby-source-graphql is something like:

myFieldName{
   pages {
      nodes {
        slug
      }
   }
}

What I need is:

myFieldName(language: $language) {
   # This part of the query is delegated to the original source with an header containing the $language value
   pages {
      nodes {
        slug
      }
   }
}

where the delegation in my case means simply to query using a particular header for the language in the underlying http call. That's the real problem in my opinion.
I really cannot see how createLink can make it possible to access the query parameter to dynamically change options, without reimplementing apollo-link-http, which seems much more complicated and hacky than what I already did (just look at the source of the plugin I posted).
But maybe I missed something. Could elaborate a simple snippet to explain better?

@vladar
Copy link
Contributor

vladar commented May 27, 2020

I really cannot see how createLink can make it possible to access the query parameter to dynamically change options, without reimplementing apollo-link-http

Apollo links compose. So, initially I thought about something as simple as this:

const { ApolloLink } = require(`apollo-link`)
const { createHttpLink } = require(`apollo-link-http`)
const fetch = require(`node-fetch`)

const myHeadersLink = new ApolloLink((operation, forward) => {
  const { query, variables } = operation

  if (variables.language) {
    operation.setContext(
      ({ headers }) => ({
        headers: Object.assign(headers, { language: variables.language })
      })
    );
  }
  return forward(operation);
})

function createLink(options) {
  const { url: uri, fetchOptions } = options
  return ApolloLink.from([
    myHeadersLink,
    createHttpLink({ fetch, uri, fetchOptions }),
  ]);
}

But now I see that the bigger challenge is defining and passing the $language variable downstream as we don't support this.

I think the most customizable solution would be to add transformSchema option so that you could apply schema transformations manually (we could pass our custom link, resolver and a list of default transforms to it).

If it is not set, we apply our default transforms. This should be a good-enough escape hatch for any custom transformations.

Will it work for you?

@ariadne-github
Copy link
Contributor Author

Thank you for your explanation, that sounds like a very good solution, also more flexible and efficient.
So, you are saying that if you add that option to gatsby-source-graphql I'd be able to tweak its default schema to add my language parameter, basically replacing the call to NamespaceUnderFieldTransform

new NamespaceUnderFieldTransform({
with a custom version that also adds the parameter.
Then I'd be able to access its value dynamically by using a custom apollo-link like in the snipped you posted.
Did I understand correctly? If that's the case, that would be perfect.

@vladar
Copy link
Contributor

vladar commented Jun 4, 2020

Yeah, that's the idea. If you are interested in this, feel free to submit a PR for this new option and tag me.

@vladar vladar added the help wanted Issue with a clear description that the community can help with. label Jun 4, 2020
@yaacovCR
Copy link
Contributor

yaacovCR commented Jun 9, 2020

@vladar @ariadne-github Can you just modify the query, variables, as desired within the link to add the field in question, and call forward(operation) on the updated operation?

Apollo links are composable as above, with access to query, variables, headers, etc, as shown above.

@ariadne-github
Copy link
Contributor Author

@yaacovCR in my understanding, there are two problems here:

  1. Being able to add an argument to the fieldName field created by gatsby-source-graphql, to allow me to pass the parameter value from the page queries.
  2. Being able to intercept that parameter from the link, and add a corrispondent header to the http call.

The first one is achievable by allowing a custom transform as a plugin option. I'm tring it by using a fork of gatsby-source-graphql as a custom plugin, and it doesn't seem difficult.
The second one is more tricky. In my tests, the snippet from @vladar doesn't work: the variables field inside operation doesn't contain neither the name nor the value of the added argument, even if the schema transform works and I'm able to insert the new argument in the query.

The solution I found for now is to tweak my custom transform to attach the args to the resolver context:

const newQuery = new GraphQLObjectType({
  name: query.name,
  fields: {
	[this.fieldName]: {
	  args: {
		[this.paramName]: {
		  type: new GraphQLNonNull(GraphQLString)
		}
	  },
	  type: new GraphQLNonNull(nestedType),
	  resolve: (parent, args, context, info) => {
		if (this.resolver) {
		  context.__args = args;
		  return this.resolver(parent, args, context, info)
		} else {
		  return {}
		}
	  },
	},
  },
})

Than I'm able to get it inside the link using operation.getContext().graphqlContext.

If it's necessary, I may try to put a working example on a public repository. In the meantime, are you able to tell me if I'm on the right track or I'm missing a simpler obious approach?

@yaacovCR
Copy link
Contributor

I reread @vladar comment above, and now I understand what you both are saying and agree with that approach. Apologies for my confusion. If you post a copy of your custom transform, I can try to take a look to see why it is not working. Best not to add stuff to context if not necessary.....

@ariadne-github
Copy link
Contributor Author

I reread @vladar comment above, and now I understand what you both are saying and agree with that approach. Apologies for my confusion. If you post a copy of your custom transform, I can try to take a look to see why it is not working. Best not to add stuff to context if not necessary.....

I agree, I'm aware that it's an escape hatch, I'm looking forward to a better solution.

Anyway, my custom transform is exactly the same as the default, except for two things:

  • I replaced the default link with:
function createLink(options) {
  return ApolloLink.from([
    myHeadersLink(options),
    link
  ]);
}

where link is the default link, and myHeadersLink is the snippet by @vladar, inside which I'm trying to access my custom parameter value and modify the headers accordingly.

  • I tweaked the NameSpaceUnderFieldTransform, as now contains an additional paramName field:
class NamespaceUnderFieldTransform {
  constructor({ typeName, fieldName, paramName, resolver }) {
    this.typeName = typeName
    this.fieldName = fieldName
    this.paramName = paramName;
    this.resolver = resolver
  }

  transformSchema(schema) {
    const query = schema.getQueryType()

    const nestedType = new cloneType(query)
    nestedType.name = this.typeName

    const typeMap = schema.getTypeMap()
    typeMap[this.typeName] = nestedType

    const newQuery = new GraphQLObjectType({
      name: query.name,
      fields: {
        [this.fieldName]: {
          args: {
            [this.paramName]: {
              type: new GraphQLNonNull(GraphQLString)
            }
          },
          type: new GraphQLNonNull(nestedType),
          resolve: (parent, args, context, info) => {
            if (this.resolver) {
              context.__args = args;
              return this.resolver(parent, args, context, info)
            } else {
              return {}
            }
          },
        },
      },
    })
    typeMap[query.name] = newQuery

    return healSchema(schema)
  }
}

Removing the context line, I cannot access the paramName value inside the custom link anymore.

@yaacovCR
Copy link
Contributor

Oof. I'm double sorry, by the time your link gets the query, added fields have been stripped, as properly pointed out above. The context workaround seems very reasonable now. Thank you for indulging me and apologies for my slowness.

@vladar
Copy link
Contributor

vladar commented Jun 10, 2020

@yaacovCR I've been thinking about passing this variable through in extensions but some of the schema transformers in graphql-tools don't pass extensions along.

For example here: https://github.com/ardatan/graphql-tools/blob/5b93e0e5d92ed2282da874175803449e34067c10/packages/wrap/src/transforms/RenameTypes.ts#L112-L115

As you can see only document and variables are passed (no extensions). Don't quite remember if others are doing the same.

@yaacovCR
Copy link
Contributor

Wo! That should be fixed! Thanks!

yaacovCR added a commit to ardatan/graphql-tools that referenced this issue Jun 11, 2020
yaacovCR added a commit to ardatan/graphql-tools that referenced this issue Jun 11, 2020
yaacovCR added a commit to ardatan/graphql-tools that referenced this issue Jun 11, 2020
yaacovCR added a commit to ardatan/graphql-tools that referenced this issue Jun 11, 2020
ariadne-github added a commit to ariadne-github/gatsby that referenced this issue Jun 17, 2020
Should resolve gatsbyjs#23552. Adds an optional custom transfomSchema option to gatsby-source-graphql that passes the default transforms, schema and link. I has many potential usages, but the consumer should be able to use the same default transforms or tweak or replace them completely. The original use case is to add an argument to the generated "fieldName" field that can then be read by a custom apollo-link to customize the underlying http call with a particular header. Tagging @vladar. This is a proposed implementation that should be safe when not used. I'm not sure if other parameters might be passed along the transformSchema arguments, maybe plugin options?
gatsbybot pushed a commit that referenced this issue Aug 6, 2020
* [gatsby-source-graphql] add transfomSchema option

Should resolve #23552. Adds an optional custom transfomSchema option to gatsby-source-graphql that passes the default transforms, schema and link. I has many potential usages, but the consumer should be able to use the same default transforms or tweak or replace them completely. The original use case is to add an argument to the generated "fieldName" field that can then be read by a custom apollo-link to customize the underlying http call with a particular header. Tagging @vladar. This is a proposed implementation that should be safe when not used. I'm not sure if other parameters might be passed along the transformSchema arguments, maybe plugin options?

* Fix implementation, renamed option

* Added options parameter and readme example

* Removed example + default implementation + references

* Renamed transformSchema in customWrapSchemaFn

* Renamed back option into "transformSchema"

Co-authored-by: Stefano De Pace <depace@ARIADNEAD.DOM>
Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue with a clear description that the community can help with. topic: GraphQL Related to Gatsby's GraphQL layer type: question or discussion Issue discussing or asking a question about Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants