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

RFC: Add APQ support for Contentful (CMS) #3415

Closed
soleo opened this issue Oct 24, 2023 · 4 comments
Closed

RFC: Add APQ support for Contentful (CMS) #3415

soleo opened this issue Oct 24, 2023 · 4 comments
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release

Comments

@soleo
Copy link

soleo commented Oct 24, 2023

Summary

Contentful GraphQL API started Automatic Persisted Query support recently, https://www.contentful.com/developers/changelog/#automatic-persisted-queries-support-in-the-graphql-api. However, their way to triggering re-fetch for missing persisted query is different from Apollo's implementation.

Contenful:
PERSISTED_QUERY_NOT_FOUND as error code https://www.contentful.com/developers/docs/references/graphql/#/reference/automatic-persisted-queries

Apollo:

{
  errors: [
    { message: 'PersistedQueryNotFound' }
  ]
}

https://www.apollographql.com/docs/react/api/link/persisted-queries/

Proposed Solution

const isPersistedMiss = (error: CombinedError): boolean =>
  error.graphQLErrors.some(
    x =>
      x.message === 'PersistedQueryNotFound' ||
      (typeof x.extensions === 'object' &&
        x.extensions !== null &&
        'contentful' in x.extensions &&
        (x.extensions as unknown as ContentfulExtensions).contentful.code ===
          'PERSISTED_QUERY_NOT_FOUND')
  );

Requirements

A better way is to propose a standardized way to utilize a set of Error Code that all vendors should be using as part of GraphQL Spec

@soleo soleo added the future 🔮 An enhancement or feature proposal that will be addressed after the next release label Oct 24, 2023
@kitten
Copy link
Member

kitten commented Oct 25, 2023

I'm actually not sure it's appropriate for us to support this. To be honest, what stops any given GraphQL HTTP API to implement any response code or shape they want? Especially in this case, Contentful has decided to namespace the error code under a custom extension key, instead of following the defacto standard.

This is even more odd in my opinion based on their request input shape following the defacto spec.

Overall, most APIs currently follow the defacto spec, and until a spec is ratified and implemented by the GraphQL over HTTP spec extensions, I don't think there's a good argument to be made for implementing arbitrary spec divergences in the urql repo.

There's of course an argument to be made for these functions to be configurable:

const isPersistedMiss = (error: CombinedError): boolean =>
error.graphQLErrors.some(x => x.message === 'PersistedQueryNotFound');
const isPersistedUnsupported = (error: CombinedError): boolean =>
error.graphQLErrors.some(x => x.message === 'PersistedQueryNotSupported');

However, again, this hasn't happened because we for now have a defacto spec, and there's no point in diverging from it just sometimes.

So, for now, I can only offer you to please "fork" @urql/exchange-persisted, or rather vendor the exchange into your codebase with the necessary changes

Hope that makes sense to you. Happy to answer more questions about it, but I think for now let's put the PR on hold and I don't think there'll be much movement in urql re. APQ support unless a new spec arises for it in GraphQL over HTTP

@soleo
Copy link
Author

soleo commented Oct 25, 2023

Thanks @kitten for your comment. We also submitted support ticket for Contentful to take a look to align with popular GraphQL client out there, e.g Relay, ApolloClient. For now, we will use my own fork.

It would be nice that the error could be part of graphQL spec though.

@JoviDeCroock
Copy link
Collaborator

I am going to close this out as it's basically a vendor-specific ask 😅

@soleo
Copy link
Author

soleo commented Oct 31, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants