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

Problems with optimisticResponse and reducer updating a list query #1100

Closed
codepunkt opened this issue Dec 28, 2016 · 5 comments · Fixed by #1144
Closed

Problems with optimisticResponse and reducer updating a list query #1100

codepunkt opened this issue Dec 28, 2016 · 5 comments · Fixed by #1144
Assignees

Comments

@codepunkt
Copy link

I've tried to reduce my example to the minimum. Basically, i have this simple schema set up on the server side:

const typeDefs = [`
  type Query {
    items: [Item]
  }

  type Mutation {
    addItem(name: String!): Item
  }

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

  schema {
    mutation: Mutation
    query: Query
  }
`];

Item ids are auto-incremented integers. All items are displayed in a list, using the item query defined in the schema above to connect data. When the loading field is set, a loader is displayed instead of the list. Also, a reducer is used to update the query result whenever a result from addItem mutation is incoming:

import React from 'react';
import { graphql } from 'react-apollo';
import gql from 'graphql-tag';
import update from 'immutability-helper';

function Item({ item: { id, name } }) {
  return <li><div>{name}</div><div>ID {id}</div></li>;
}

function ItemList({ data: { items, loading } }) {
  if (loading) return <div>loading...</div>;
  return <ul>{items.map(item => <Item key={item.id} item={item} />)}</ul>;
}

const itemQuery = gql`
  query items {
    items { id name }
  }
`;

export default graphql(itemQuery, {
  options() {
    return {
      reducer: (prev, { operationName, type, result: { data } }) => {
        if (type === 'APOLLO_MUTATION_RESULT' && operationName === 'addItem' && data) {
          return update(prev, { items: { $push: [data.addItem] } });
        }
        return prev;
      },
    };
  },
})(ItemList);

Right below the item list is a simple form used to add new items to the list. This addItem mutation invoked by this form is set up with an optimisticReponse that generates a temporary uuid as the items id (instead of the server generated auto-incrementing integers):

import React from 'react';
import uuid from 'uuid/v4';
import gql from 'graphql-tag';
import { graphql } from 'react-apollo';

class ItemForm extends React.Component {
  constructor(props) {
    super(props);
    this.state = { name: '' };

    this.handleNameChange = this.handleNameChange.bind(this);
    this.handleSubmit = this.handleSubmit.bind(this);
  }

  handleNameChange(event) {
    this.setState({ name: event.target.value });
  }

  handleSubmit(event) {
    this.props.add(this.state.name);
    this.setState({ name: '' });
    event.preventDefault();
  }

  render() {
    return (
      <form onSubmit={this.handleSubmit}>
        <input type="text" name="name" value={this.state.name} onChange={this.handleNameChange} />
        <input type="submit" value="Add Item" />
      </form>
    );
  }
}

const addItem = gql`
  mutation addItem($name: String!) {
    addItem(name: $name) {
      id name
    }
  }
`;

export default graphql(addItem, {
  props: ({ mutate }) => ({
    add: name => mutate({
      variables: { name },
      optimisticResponse: {
        __typename: 'Mutation',
        addItem: {
          name,
          __typename: 'Item',
          id: uuid(),
        },
      },
    }),
  }),
})(ItemForm);

To be able to test things out, i added an artificial delay of 5 seconds to the addItem mutation resolver on the server. When i add a single item, the optimisticResponse is immediately reflected in the item list and 5 seconds later, the server id replaces the generated temporary uuid.

When i add another item while the first mutation is not resolved, things start going south. The list already has the (temporary optimisticResponse) first added item. Once the first mutation resolves, the list goes to a waiting state - and i don't know why.

I've added a short video of this with Apollo Dev Tools opened so you can follow my writings. Whatever happens triggers the loading field. In the store, the temporary item is replaced with the server-approved item once the mutation is resolved, but the ROOT_QUERY, which is inspected in the dev tools, somehow retains the old temporary item until all stacked/parallel optimisticResponse mutations are resolved. This is reproducible with more than 2 mutations in parallel aswell.

apollo-optimisticresponse-problem

Why? What i would've expected is each item being added to the list immediately in its temporary optimisticResponse version and being replaced after 5 seconds when the server resolved the mutation - not being affected by any other mutations.

@codepunkt codepunkt changed the title Problems with staggered optimisticResponse combined with reducer live-updating a list Problems with optimisticResponse and reducer updating a list query Dec 28, 2016
@helfer
Copy link
Contributor

helfer commented Dec 30, 2016

@codepunkt Thanks for the really thorough problem description! In short, what I read is that the following happens:

  1. You make a first mutation with optimistic update, the first optimistic item is shown
  2. You make a second mutation with optimistic update, both optimistic items are shown
  3. The first mutation resolves with the actual result, but your UI now shows the loading state
  4. The second mutation resolves with the actual result, but the UI still shows the loading state.

I'm not 100% sure about number 4. Does it stay in loading after all mutations are resolved, or does the UI actually display the real mutation results?

What you found sounds like it could be related to #1039, but in your case I'm 99% sure that the problem is due to some bug in Apollo Client. But just out of curiosity, could you try running your example with $unshift instead of $push. I don't think it should make a difference, but if it did, it would be a useful piece of information.

Is there any chance that you could make a small github repo with your reproduction? That would be super helpful, because I could then run it with a debugger and see what's really happening inside Apollo Client.

@codepunkt
Copy link
Author

@helfer Thanks for looking into this - very appreciated!

It does not stay in loading after all mutations are resolved - it displays the mutation results. Something weird seems to happen in the store. Try to look at the attached gif a few times and you will notice that once the first of two mutations resolves, the resulting item is duplicated in the store because the optimisticResponse item still persists even though the item from the server resolve was received.

Using $unshift instead of $push has the same problem.

You can find a repo with my reproduction at
https://github.com/codepunkt/apollo-optimisticresult-problem-reproduction/

@codepunkt
Copy link
Author

Couldn't solve this as of now. Anyone else got any ideas?

@calebmer
Copy link
Contributor

calebmer commented Jan 7, 2017

Found the bug and I submit a PR to fix it. The PR explains the cause and solution: #1144

Haven’t addressed the tests yet, only got your failing case to work @codepunkt. May need to wait until next week to see this merged, but a solution is on the way 👍

@codepunkt
Copy link
Author

@calebmer @helfer Amazing, thanks a lot!

@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 a pull request may close this issue.

3 participants