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

Referencing fragments and queries #480

Closed
Poincare opened this issue Jul 29, 2016 · 11 comments
Closed

Referencing fragments and queries #480

Poincare opened this issue Jul 29, 2016 · 11 comments

Comments

@Poincare
Copy link
Contributor

Poincare commented Jul 29, 2016

Currently, the way that we reference queries and fragments is pretty splintered and leaves the API feeling somewhat incohesive. There are multiple ways to reference both queries and fragments, depending on circumstance. For example, updateQueries refers to queries through query names but fragments are passed around using the return value of createFragment rather than by fragment name. Here's an outline of an idea we can use to unify this as well as introduce the ability to reference queries from other components and refetch them, start polling on them, unsubscribe from them, etc.

Design

The core idea is that we should always reference fragments and queries by their names. This makes sense because this is how we reference them within GraphQL itself so we don't have to introduce another layer of complexity by passing around instances of these documents rather than just reference them by name.

getQuery API

The method getQuery that look up a query with its name. For example, this may look like:

client.query(gql`
query authorNames {
  author {
    firstName
    lastName
  }
}`);

// later on, in some totally different place
getQuery('authorNames').refetch();

To accomplish this, every time query or watchQuery is called with a particular document, it will insert it into a map going from the name of the query to the query id created for it inside QueryManager. Then, methods like refetch, startPolling are called on the return value of getQuery, they can be applied to every ObservableQuery and Promise that has been issued for that particular query.

Since this is information we already have to keep track of in order to reset the store correctly, this doesn't need any additional data structures other than the map from query name to query id.

updateQueries API

Since we would always use names to reference queries, we don't have to change the updateQueries API in any way.

Fragments API

Rather than passing around the return value of createFragment and also using the fragment name within the query document to reference the fragment (needless duplication), we can always reference the fragment by name. This is what it could look like:

createFragment(gql`
    fragment personDetails on Person {
       name
   }`
);

// much later, in some other component
client.query(gql`
  query {
    person {
      ...personDetails
    }
  });

This also involves a map from fragment name to the fragment definition. This map will be used to resolve the personDetails fragment spread within the query.

Why global names

There is an obvious reason to not do any of this: we're essentially introducing a single namespace each for fragments and queries. However, there are several reasons that doing this actually makes sense.

Fundamentally, GraphQL presents a single namespace and all references are made by name (e.g. fragment spreads). Since this is the case, if we were to adopt anything other than fragment names, we'd have to end up using fragment names within the GraphQL documents any way.

We could attempt to solve this problem by using string interpolation when constructing GraphQL documents and introducing helper utilities that translate a fragment object into a name. But, this would make queries in your editor pretty different from what gets sent to your server and would make static analysis for queries much more difficult. Given these reasons, I think it makes sense to create a namespace for fragment and query names and warn the application developer in case there are conflicts among them.

Feedback would be appreciated!

@helfer
Copy link
Contributor

helfer commented Jul 29, 2016

I mostly agree with your proposal. It would be awkward if a fragment or query could have a different name in the document than the one we use to reference it by.

Currently it looks a bit weird because it seems like graphql queries and fragments are just magic strings, but I think it could look much cleaner if the queries and fragments were put into .gql files, which would (in most cases) remove the need for the gql tag and for calling createFragment.

@Poincare
Copy link
Contributor Author

I think the general guideline is for queries to be placed within the same file as the component declaration so createFragment and gql will likely serve as the most common use case.

@helfer
Copy link
Contributor

helfer commented Jul 29, 2016

That would speak against having a global namespace then, in my opinion.

@Poincare
Copy link
Contributor Author

Poincare commented Jul 29, 2016

I'm not sure if there's a way to accomplish this more effectively. We'll have to reference the fragments by name in any case; passing around references to the fragments and also having to reference them by name is a pain.

Using this approach, we are basically reflecting the "global namespace" that GraphQL has. There is some discussion in the graphql repo and otherwise that says GraphQL may soon support namespaces, which would mean that this solution would work with these namespaces pretty much immediately.

@helfer
Copy link
Contributor

helfer commented Jul 29, 2016

Yeah, since queries and fragments already have names, I think it makes sense to use those. We'll have to force people to choose names though, and make sure that they are unique. If we go down that path, I'm guessing that we'll soon be interested in some sort of namespacing as well.

@jbaxleyiii
Copy link
Contributor

@helfer @Poincare in the new react-apollo, I'm using query/mutation names as the key in the generated props. We also moved our queries into .gql files which get importes. I like the global namespace because it is just graphql at that point. It also mirrors using the schema language to generate the schema on the server to some extent

@Poincare
Copy link
Contributor Author

Poincare commented Jul 29, 2016

Ok, I think this makes sense from most perspectives. I agree that namespacing would be nice if we could have it - until then, I'll go ahead and work on implementing this within Apollo Client.

@peisenmann
Copy link

I know I'm commenting on a pretty old issue here, but after a couple of hours searching the current code base and issues, I can't find anywhere that this was implemented.

Did the idea of a global fragment map get canned, or is it implemented in a way that I was too dense to absorb it in the docs?

@stubailo
Copy link
Contributor

We've actually since decided to move away from the idea of global queries, since it didn't seem to work too well with JavaScript conventions. What are you trying to do?

@peisenmann
Copy link

peisenmann commented Apr 3, 2017

@stubailo Thanks for the response. I was attempting to setup something with global fragments (not really global queries, per se) where I could do

@graphql(gql`
query myDataQuery {
  people {
    ...StandardPerson
  }
  companies {
    ...SimpleCompany
  }
}
`);

Without having to pipe in the strings for the fragments into every query like ${fragments.people.standard} ${fragments.company.simple}.

I was able to more or less achieve my goal by running all my fragments through the graphql tag, and pulling out all the FragmentDefinitions and storing them in a map by their name. Then, later as Apollo client middleware, I take the query document, recursively sift through the selections looking for FragmentSpreads and pull those definitions from the map and add them to the definitions of the query doc.

I do realize that creates a global namespace of fragments, and that could potentially be an issue with third party tools and every other reason that we don't do global in JS normally. The upside is not having to import and inject the query text every where those fragments are used.

Definitely open to alternatives and suggestions.

@caub
Copy link
Contributor

caub commented Mar 14, 2018

Same here, I wonder why it wouldn't be possible to send smaller queries by dropping the repetitive (across queries) fragment definitions, and refer to fragments by their name. Fragment can be defined on the server as well, and shared with client, for validation and knowing what is in.

Is it something possible at Apollo level, or at Graphql level?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants