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

GraphQL Enhancements #94

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

LoganSmithDarkLight
Copy link

Fetchye currently creates a new cache for every GraphQL query fired (assuming the queries have minor differences). Consider the following scenarios:

  • If a query requests a subset of fields already requested by a previous query, an entirely new api call is made and stored in cache.
  • If a query only requests one or two new fields and many already requested fields, an new api call is again made with the full set of query fields.

This change aims to prevent duplication in the way Fetchye requests data from GraphQL APIs. The goals of this change are as follows:

  • A new call is made to a GraphQL API only if the query requests new fields. Otherwise, it pulls existing data from cache.
  • If a query only requests a few new fields, all already existing fields are stripped from the request.
  • If the query includes different arguments, naming, etc that could impact the response of the GraphQL API, a new cache is created for that request.
  • This change should have an opt-in / opt-out strategy so that it does not impact existing Fetchye users, and is not mandatory to use if the assumptions (listed at the bottom) do not apply to your use case.

The following goals are accomplished by introducing these changes:

  • A "query" field has been added to the store (alongside isLoading, data, and error).
  • A new field can be passed to options named "isGraphQL". Adding this field as true will opt-in to these changes.
  • Whenever a GraphQL call is made to Fetchye, the query in the body is trimmed down to only the special GraphQL fields before the hash key is created. This way, the hash key is always the same for all GraphQL apis using the same special fields.
  • Whenever a GraphQL call is sent to Fetchye, the query is pulled from the body and dispatched to the store. Subsequent queries are merged and compared against the query already present. If any new fields have been added, an API call is made. Otherwise, Fetchye responds with data from the cache.
  • Before the API call is made, any existing query fields are stripped from the body (with the exception of required special fields).
  • GraphQL responses are merged with the data in the cache rather than replacing it. This allows already existing fields to be served.

The following assumptions are made to ensure this process can function:

  • A "query" field is present in the request body.
  • The "query" field is formatted using json-to-graphql-query. (Necessary for object comparison.)
  • The response from the API is stable, and can be safely merged.

Potential issues identified with this approach:

  • Because data is merged, Fetchye will respond with the full list of fields requested by all GraphQL calls. If developers blindly use fields that were not part of a specific request, they may introduce dependency issues.
  • Comparisons and merging use deep compare and deep merge functions which can be costly. These functions are currently only running when data changes (not on every render), so the impact may not be significant.
  • An error given by any of the GraphQL requests would reflect to all of them. This PR is open to suggestions on how to handle those scenarios.

Thank you for your time in reviewing this! If you have any concerns or questions please leave a comment. I also have a working demo of this change if you would like to see it. Please reach out to me if so.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


lpsmit seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@oneamexbot
Copy link
Contributor

This pull request is stale because it has been open 30 days with no activity.

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

Successfully merging this pull request may close these issues.

3 participants