apollographql / apollo-feature-requests

🧑‍🚀 Apollo Client Feature Requests | (no 🐛 please).
Other
130 stars 7 forks source link

Pass `extensions` to the client #117

Open bsara opened 5 years ago

bsara commented 5 years ago

Migrated from: apollographql/react-apollo#2059

adamscybot commented 5 years ago

Continuing coversation from apollographql/react-apollo#2059. I got around this by adding a link that overloads data with a function that returns the extensions.

// See https://github.com/apollographql/react-apollo/issues/2059
// Because of this we have to surface extensions via data to make them
// accessible downstream.
const ForwardExtensionsLink = new ApolloLink((operation, forward) => {
  return new Observable(observer => {
    const sub = forward(operation).subscribe({
      next: result => {
        result.data.extensions = () => result.extensions
        observer.next(result)
      },
      complete: observer.complete.bind(observer),
    })
    return () => {
      if (sub) sub.unsubscribe()
    }
  })
})

Then

    <Query
       ....
    >
      {result => {
        console.log(
          ' result',
          result.data && result.data.extensions && result.data.extensions()
        )
    <Query>
sgrove commented 5 years ago

We have a similar approach we've taken that's slightly less elegant (using a custom link + mutable state outside the request): https://github.com/OneGraph/onegraph-examples/compare/request-metrics

We wrote about the motivating case a bit as well

nathanburgess commented 5 years ago

In case anyone else comes across this thread: you can simply access error.graphQLErrors now.

const { error } = useQuery(SOME_QUERY)

if (error)
    console.log(error.graphQLErrors)
Borvik commented 5 years ago

@nathanburgess While correct you can access it via the errors, the problem is accessing the extensions when there isn't an error.

nathanburgess commented 5 years ago

@nathanburgess While correct you can access it via the errors, the problem is accessing the extensions when there isn't an error.

Ah, my bad, I misinterpreted the issue then.

Borvik commented 4 years ago

@Migushthe2nd Where did you see they are "moving away from extensions"?

As far as I can tell they are deprecating the graphl-extensions in favor of a Plugin system. Doesn't mean this extension in the response, which is what this is all referring to, will be going away.

Migushthe2nd commented 4 years ago

@Borvik I might have mixed up some some terms, probably because I can't find a good overview. I'd appreciate it if you could help answer a few questions :)

  1. So this graphql-extensions, what does it do? I can't find any documentation on it (anymore).
  2. What is the difference between graphl-extensions and the new Plugin system?
  3. What is the difference between GraphQL's extensions and Apollo Server's extensions?
  4. What is the extensions field in ApolloServer used for?
Borvik commented 4 years ago

Not sure I can fully answer that, but I'll give it a shot (barely got my feet wet with either).

graphql-extensions was I believe an informal (as in not a formalized spec for how apollo-server should work) for extending the server functionality by adding. It allowed tapping into various points of the request lifecycle, such as before the resolvers, after the resolvers, on startup - just before the response to the client. Without the docs I can't be sure of exactly which points it allowed hitting.

The new Plugin system - does the same thing, it's just a slightly different format, and formalized it making it no longer "experimental". See https://www.apollographql.com/docs/apollo-server/integrations/plugins/

Not 100% sure what you mean with 3, but the only extensions I am aware of as part of the GraphQL spec deal with the schema and extending (inheriting) types.

As far as the extensions field, I'm assuming you mean what is sent in the response. I don't know if that's an apollo spec, or GraphQL spec, but it just a way of returning extra data not part of the main query/mutation being run. Incidental things, like apollo-tracing providing performance metrics of the queries.

Borvik commented 4 years ago

Incidentally I just looked at your linked issue for vue-apollo, and that is one of the main reasons I have looked into it - pagination data. Didn't want to bloat the types with pagination stuff.

Other scenarios - alerts. I've got something set up for the server to log an alert (success, warn, info, error) that will cause a toast to show on the client. It works in some scenarios, but not all - because extensions get filtered in some cases.

Another scenario - permission lists. Really in a query it would be nice to pass a permission list of permissible actions for a given object/page. This permission list could help drive the UI of a client, and doesn't really belong in the type (say a Product) as it doesn't really belong to the Product but rather the permissions of the logged in user of what they may do with any Product (vs a specific product - like is_locked, which indeed would belong on the type).

Migushthe2nd commented 4 years ago

Those three cases you're talking about, do you use vue-apollo in their project(s)? I'm asking since I don't think this is an apollographql issue anymore, because of https://github.com/apollographql/apollo-feature-requests/issues/117#issuecomment-514256660. From what I have tested the vue-apollo module seems to be the cause and I think it's better to continue discussion there (https://github.com/vuejs/vue-apollo/issues/1018) if you don't mind.

Borvik commented 4 years ago

Actually I don't use vue-apollo - I use react-apollo.

If I remember tracing it properly, I think it actually traced back to apollo-client

Migushthe2nd commented 4 years ago

Oh you you did (unlike me)? I'll have a look at it tomorrow. Thanks!

Migushthe2nd commented 4 years ago

Okay this seems quite hard to implement properly. I tried two ways to include metadata (via formatResponse in ApolloServer):

My formatResponse: image And then how I access extra data there (an example): image It works I guess

There are problems with both of them.

As data.pagination

For this is that apollo-client is set-up so that is uses graphql query documents to compare cache entries. The received data is filtered so that it only includes all fields specified in the graphql query.

A few things I thought of:

Possible solution(s):

As extensions.pagination

For this the apollo-client module, for me vue-apollo, for you react-apollo, etc. would have to be updated as almost everywhere only the data field read.

Possible solution(s):

Solution

For now, creating a "fake" query seems to work the best and easiest. Do note that if your API is public, you should document this and explain clearly it is not a real query. This does not require https://github.com/apollographql/apollo-feature-requests/issues/117#issuecomment-514256660. On your Apollo Server create a new type of the metadata you want: image

Then create a new query e.g. pagination with the type: image

Go to your client and edit the the query you call in Vue or React. Add your metadata query with their fields: image

The data is not not filtered from the response anymore, and this even seems to work with caching (why I think I might be wrong about something I wrote earlier)! Edit: the two queries aren't cached together. To fix this, I make a simple hash of the main query variables and send it as a variable to the pagination query. I know, this increases the upload payload again, but in my case I prefer this: image

With nuxt-apollo (idk about others), if you look at the $apolloData object in the vue devtools, you'll see it doesn't include the pagination data: image

To fix this, use the update function and just return all data: image

The $apolloData object will then look like: image

Borvik commented 4 years ago

Glad you found a solution for you - also you didn't show how the data is being passed back from the resolver.

Not much different from doing this for the server schema:

type Product {
  id: ID!
  name: String!
}

type ProductList {
  products: [Product!]
  pagination: Pagination
}

extend type Query {
  getProductList(): ProductList
}

Really I think most just want the extension data passed along - yes it would mean modifying a bunch of stuff.

Looking over my alerts code - I don't think there is an issue with it, I don't actually need it in the specific page the query is on, and an ApolloLink still has access to the extension data for the alerts and raises them fine.

I think the other scenarios are still valid reasons to pass extensions. Shoot, perhaps the application being developed actually wants to show performance metrics to the user (very limited use case I know, but not outside the realm of possibility) - in which case we should be able to see the extension data in <Query> results (or await client.query() calls).

To me at least - I'd say it was a bug that it isn't in there to begin with. Apparently the apollo-bot thought otherwise and flag it as a feature, and eventually it got transferred here to the feature requests.

Migushthe2nd commented 4 years ago

(I edited my message above to show how I pass data along from the resolver and because the pagination query wasn't cached together with the main query correctly)

Yeah, you're right that I'm basically back where I started 😂, but my current solution will be fine for me for the time being. In my project I'm using JoinMonster (a module which can fetch data almost directly from the DB), which doesn't play nice with extra fields in the root of the query it can't fetch from the database. That was mainly the reason I didn't want to include the type in the query I sending to the backend. Now that I am basically sending a second query, JoinMonster can ignore it (it can ignore a full query, but it can't properly ignore a root field in a query).

Anyway, I fully agree with you. The extension data should simply be included in the results.

NickTomlin commented 4 years ago

I've been bitten by this recently as well. @bsara approach does work, but only if fetchPolicy: "no-cache" is specified; . apparently this is a known limitation/bug that hasn't been fixed or documented yet

I've simplified the link code slightly:

new ApolloLink((operation, forward) =>
  forward(operation).map(response => {
    if (response.data) {
      response.data.extensions = response.extensions
    }
    return response
   })
).concat(new HttpLink())

And this is the required apollo code to make it work:

const {data} = useQuery(gql`query { ping }`, {
    fetchPolicy: "no-cache"
})

if (data) {
  console.log(data.extensions)
}

Disabling the cache is not ideal at all; i'm not sure if this is because it is impossible or difficult to handle the untyped nature of extensions, but ideally there would be a way to allow this to pass through without disabling the cache entirely.

MatthiasKunnen commented 3 years ago

Could a maintainer confirm that a PR which passes extensions through in the FetchResult will be considered? The extensions field will be cached.