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

Inject apollo-client into operation on network layer #3877

Closed
wants to merge 3 commits into from
Closed

Inject apollo-client into operation on network layer #3877

wants to merge 3 commits into from

Conversation

lucasfeliciano
Copy link

@lucasfeliciano lucasfeliciano commented Sep 3, 2018

Why?

I was implementing global error handling and I realise that best place to do that would be in the network layer, however it doesn't have access to apollo-client.

The reason that I think this is super beneficial is because we can perform mutations at the network layer. Specially if you want to update a link-state.

Motivation

After searching solutions for the my problem I ended up in this issue: apollographql/apollo-link#527

Which is exactly what I was thinking of so I decided to push this forward.

Example

First we defined a resolver in our link-state

showErrorFeedback: (_, variables, { cache }) => {
  const data = {
    errorFeedback: {
      __typename: 'ErrorFeedback',
      show: true,
      error: variables.error,
    },
  }
  cache.writeData({ data })
  return null
}

Then we defined our mutation:

export const SHOW_ERROR_FEEDBACK = gql`
  mutation showErrorFeedback($error: Object!) {
    showErrorFeedback(error: $error) @client {
      show
      error
    }
  }
`

Finally, we can perform this mutation at the network layer level:

const errorHandlerAfterware = onError(({ networkError, operation }) => {
  if (networkError && networkError.statusCode === 500) {
    operation.client.mutate({
      mutation: SHOW_ERROR_FEEDBACK, 
      variables: { 
        error: new Error('Something went wrong')
      }
    })
  }
})

With that we can have a component that receive the link-state and show the respective error in a global level.

To discuss

I still have to add client: ApolloClient type definition into Operation type, but I realise that this come from apollo-link, so I don't know the best way of doing that.

I would appreciate some help/directions on the best approach of doing that.

@apollo-cla
Copy link

@lucasfeliciano: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@hwillson hwillson self-assigned this Nov 14, 2018
@hwillson
Copy link
Member

hwillson commented Dec 12, 2018

First off, thanks very much for working on this PR @lucasfeliciano!

As it stands currently, the ApolloClient constructor is already getting a bit out of hand in terms of everything it's trying to cram in, especially with regards to manipulating the link chain. Taking a step back a bit, I think we need to question why that is, and why we're not allowing the link chain to be directly manipulated outside of ApolloClient instantiation. If we opened up the public Apollo Client API with setLink / getLink, you'd be able to add the client instance (via a link) pretty easily yourself, after instantiation. This would of course open up many other possibilities as well, like being able to dynamically modify the link chain throughout an application, based on external factors.

There isn't really anything stopping us from allowing the link chain to be manipulated outside of the constructor. We're adding a supportedDirectives link to the chain in the constructor, which we need and we wouldn't want people to be able to remove, but we can handle this elsewhere. We could add it to the link chain just before the chain is executed in the QueryManager instead. Doing that would mean we can add link getters/setters to the ApolloClient API, let people do whatever they want with the chain at any point, and still preserve the additional links we need internally.

We'll think about this a bit more, and post back here shortly. Thanks!

@hwillson
Copy link
Member

Just to add, if we opened up the API a bit, a solution that replicates the functionality in this PR would look something like:

const client = new ApolloClient({
  cache: new InMemoryCache(),
});

const link = ApolloLink.from([
  new ApolloLink((operation: Operation, forward: NextLink) => {
    operation.client = client;
    return forward(operation);
  }),
  // ... the rest of your link chain ...
]);

client.setLink(link);

@lucasfeliciano
Copy link
Author

lucasfeliciano commented Apr 15, 2019

Closing this PR for the more flexible solution above

@lucasfeliciano lucasfeliciano deleted the enhancement/inject-apollo-client-into-network-layer branch April 15, 2019 11:36
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 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

Successfully merging this pull request may close these issues.

3 participants