urql-graphql / urql-exchange-graphcache

A normalized and configurable cache exchange for urql
88 stars 13 forks source link

The optimistic updates are not cleared on errors #159

Closed dragoscd closed 4 years ago

dragoscd commented 4 years ago

If the server throws an unexpected error or the network connection goes down the optimistic data is not cleared.

JoviDeCroock commented 4 years ago

Hey @dragoscd

I've taken a quick look and it seems like this works in our tests, could you provide any more information. For instance do you have an exchange that re-throws this error? What data is returned together with said mutation?

If possible could you provide a minimal reproduction? This can speed up the finding of the bug in question

dragoscd commented 4 years ago

I've used the subscriptions example and added a simple mutation there:

server:

Mutation: {
    addMessage: () => {
      throw new Error('Something');

      const newMessage = {
        id: ++id,
        message: StarWars.quote(),
        from: StarWars.character(),
      };
      store.messages.push(newMessage);

      return newMessage;
    },
  }, 

client:

const GetQuery = gql`
  query {
    messages {
      id
      message
    }
  }
`;

const client = createClient({
  url: 'http://localhost:4000/graphql',
  exchanges: [
    cacheExchange({
      updates: {
        Mutation: {
          addMessage: (data, _, cache) => {
            if (data && data.addMessage) {
              cache.updateQuery({ query: GetQuery }, (prevData: any) => {

                return {
                  ...prevData,
                  messages: [...(prevData.messages as Data[]), data.addMessage],
                };
              });
            }
          },
        },
      },
      optimistic: {
        addMessage: () => {
          return {
            id: 12312,
            message: 'This is the optimistic message,
            __typename: 'Message',
          };
        },
      },
    }),
    fetchExchange,
    subscriptionExchange({
      forwardSubscription: operation => subscriptionClient.request(operation),
    }),
  ],
});
JoviDeCroock commented 4 years ago

Given that information the issue will be because this optimisticLayer is persisted by means of cache.updateQuery due to this line: https://github.com/FormidableLabs/urql-exchange-graphcache/blob/master/src/operations/write.ts#L165

let me clarify that this is intended but maybe we are incorrectly applying the layer on top of messages, this will need some debugging.

EDIT: I have identified the issue with this and it seems that when there's no data we don't correctly get our dependencies resulting in a no-update.

JoviDeCroock commented 4 years ago

Hey,

This is fixed by https://github.com/FormidableLabs/urql-exchange-graphcache/pull/160, if you run into any more issues with this feel free to reopen the issue!

dragoscd commented 4 years ago

Awesome, thanks for the quick support, I'll check it out