Repository: apollographql/apollo-client

Problem statement:
Getting Store Errors after upgrading to v1.0.0
Since upgrading from a previous release candidate (rc6), I am now getting errors when running _**ALL**_ of my create/delete mutations, regardless of what data they mutate I always get the same error:

```
Allergen.jsx:38 Error: Error: Store error: the application attempted to write an object with no provided id but the store already contains an id of Site:developer for this object.
    at writeFieldToStore (http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:49816:23)
    at http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:49709:17
    at Array.forEach (native)
    at writeSelectionSetToStore (http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:49703:29)
    at writeResultToStore (http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:49688:12)
    at http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:51277:20
    at http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:50464:34
    at Array.forEach (native)
    at data (http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:50463:43)
    at apolloReducer (http://localhost:3000/packages/modules.js?hash=c1dea742144f5f0b244684fb3a95fddf714f326e:50675:23)
(anonymous) @ Allergen.jsx:38
(anonymous) @ meteor.js?hash=27829e9…:1105
```

In this particular instance I was attempting to delete an `Allergen`.

For reference here is the relevant client-side code:
```jsx

const Allergen = ({ ..., removeAllergen }) => {
  const onDelete = () => {
    bootbox.confirm(`Are you sure that you wish to delete allergen '${allergen.name}'?`,
      (result) => {
        if (!result) return;
        removeAllergen({ id: allergen._id }).catch((methodError) => {
          toastr.error('An error occurred when attempting to delete the allergen');
          console.error('Error:', methodError);
        });
      },
    );
  };

  ...
};

const removeAllergen = gql`
  mutation removeAllergen($id: String!) {
    removeAllergen(id: $id)
  }
`;

const AllergenContainer = compose(
  graphql(removeAllergen, {
    props({ mutate }) {
      return {
        removeAllergen({ id }) {
          return mutate({
            variables: { id },
          });
        },
      };
    },
  }),
)(Allergen);

export default AllergenContainer;
``` 

and the server-side mutation:

```js
export const schema = [`
  type Allergen {
    _id: String!
    createdAt: Date
    updatedAt: Date
    site: Site!
    siteId: String!
    name: String!
  }
`];

export const mutations = `
  # Deletes the specified allergen
  removeAllergen(id: String!): String
`;

export const mutationResolvers = {
  removeAllergen(root, { id }, context) {
    if (!context.user) throw new Meteor.Error('allergens.removeAllergen.login', 'Must be logged in.');

    const allergen = Allergens.findOne({ _id: id });
    if (!allergen) throw new Meteor.Error('allergens.removeAllergen.notFound', 'The specified allergen does not exist');
    userIsOwner(allergen, context.user);

    // Remove the allergen from the database and return the id of the removed document
    const docsRemoved = Allergens.remove({ _id: id });
    return docsRemoved === 1 ? id : null;
  },
};
```


Hints:
Rolling `apollo-client` back to `v1.0.0-rc.6` makes it all work again... I'll continue testing the versions in between to try to narrow the issue down...

However I can confirm that `v1.0.0-rc.7` breaks all my mutations. So the issue was likely introduced there.
I'm starting to think that it might be related to the `reducer` functions so for completeness I'll add them too:

```jsx
import update from 'immutability-helper';

const dataQuery = gql`
  query getAllergenInfo {
    allergens {
      _id
      siteId
      name
    }
    site {
      _id
      siteSettings {
        allergenPrefix
      }
    }
  }
`;

const AllergensPageContainer = compose(
  graphql(dataQuery, {
    props: ({ data }) => {
      if (data.loading) return { loading: true };
      if (data.error) return { error: data.error };
      return {
        allergens: data.allergens,
        site: data.site,
      };
    },
    options() {
      return {
        reducer: (previousResult, action) => {
          if (action.type === 'APOLLO_MUTATION_RESULT' && action.operationName === 'insertAllergen') {
            const newAllergen = action.result.data.insertAllergen;
            return update(previousResult, {
              allergens: {
                $unshift: [newAllergen],
              },
            });
          } else if (action.type === 'APOLLO_MUTATION_RESULT' && action.operationName === 'removeAllergen') {
            const removedId = action.result.data.removeAllergen;
            if (!_.isString(removedId)) return previousResult;
            const index = _.findIndex(previousResult.allergens, { _id: removedId });
            if (index === -1) return previousResult;
            return update(previousResult, {
              allergens: {
                $splice: [[index, 1]],
              },
            });
          }
          return previousResult;
        },
      };
    },
  }),
)(AllergensPage);
```
@Siyfion could you provide a reproduction using react-apollo-error-template? That would help us find and fix the issue faster.
I saw a similar issue upgrading a client app to 1.0.0. I don't have time to give a good repro, but I thought I'd mention our code uses `reducer` code very similar to @Siyfion 's, in case that helps. We were at rc6 when we found the issue and have set the upgrade aside for the time being.
@rdickert I'm assuming you're also using `meteor-integration`, right?
I've tried to help @Siyfion solving this issue, here are which *assumptions* have come out after our discussion (*assumptions as in my knowledge of the internals of `apollo-client` is limited*).

The error happens when updating the store after a mutation succeeded: part of the reducer output conflicts with what is already there in the store, resulting in the error `Store error: ...`. 

Looking at [the code related to the error](https://github.com/apollographql/apollo-client/blob/6b6e8ded1e0f83cb134d2261a3cf7d2d9416400f/src/data/writeToStore.ts#L297-L316), we are in the case of a generated id is  already in place for the new id we want to insert. The "funny thing" is that, in the case of the issue raised, the id causing trouble isn't the added/removed result, it's an id of another type already present :sweat_smile:.

The query asks for `allergens` & `site`, the mutation updates `allergens`, the error is on `site` ([screenshot of a breakpoint just before the error](https://cl.ly/2D3P460Q2K0X)):
- the `escapedId`, already present, is `Site:developer` (`Site` is the typename, `developer` is the id);
- the new `storeValue` is generated, it's `$ROOT_QUERY.site`.

> Could the problem come from an inconsistency in the way of asking for data corresponding to `site` ? 

The query asks for `site` on the root, but also on the allergen with `siteId`. `siteId` being resolved as:
```js
export const resolvers = {
  Allergen: {
    site: ({ site }) => Sites.findOne({ _id: site }),
    siteId: _.property('site'),
  },
};
```

I don't see in the changelog what could have caused the version upgrade triggering that kind of behavior? About the `meteor-integration`, the only real thing it does is plugging a new value on the headers on every request send, so I don't exactly see how it could cause the issue.

Hope it helps!
Just to confirm that the issue still exists in `v1.0.1` and that I believe @SachaG is also suffering from this issue?
@xavcz so the error is that your mutation somehow also returns a site that "conflicts" with a site already in the cache?
@wmertens The mutation doesn't return anything at all to do with a `Site`, in fact I get an error when dealing with the `removeAllergen` mutation which only returns a simple `String`.
I can confirm I've been running into pretty clear-cut issues with mutation updates. I'm not using `meteor-integration` btw. 
I hit this error so I did 2 things:

1. I added the `addTypename` attr to my ApolloClient constructor
2. I specified __typename in all my relevant queries that the mutation touched

The fact that I had to do those things seemed weird, but bigger fish to fry on my end.
Hi I have this error after mmigrate to v1.0.1 

```Network error: Store error: the application attempted to write an object with no provided id but the store already contains an id of UserNodeVXNlck5vZGU6MTUwNjA= for this object.```

can someone help me?

Thank you!
Thanks all! A reproduction with react-apollo-error-template would really help here, but I'll see if I can figure out what's going on without it.
I can confirm @abhiaiyer91's fix seems to work for me using 1.0.1.
Ok, after thinking for bit and looking at the code I'm fairly sure I know what's going on. The issue is that `reducer` takes its query from `observableQuery.options.query`, which does not yet have the `addTypename` transform applied.
The workaround is to add `__typename` to all selection sets in queries that have a reducer. I'm not sure if `updateQueries` has the same issue (it's not trivial to check), but `update` doesn't, because I recently fixed it (I think).

The quick solution would be to apply the `addTypename` transform every time the reducer runs (it's pretty cheap).
A slightly longer term solution might be to just apply `addTypename` as soon as the document enters ApolloClient through `query` or `watchQuery`, as we do for `mutate`.
In the long term, we'd like to apply the transform only where it's really necessary, and return only the selections that the user originally requested, not including `__typename` if it wasn't asked for.

PS: It's interesting that this only showed up after we created a default `dataIdFromObject` function. I would have assumed that such a problem would show up much sooner. It guess it shows that most people weren't using dataIdFromObject.

Patch:
diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
--- a/src/core/QueryManager.ts
+++ b/src/core/QueryManager.ts
@@ -989,7 +989,7 @@ export class QueryManager {
       if (queryOptions.reducer) {
         return createStoreReducer(
           queryOptions.reducer,
-          queryOptions.query,
+          this.addTypename ? addTypenameToDocument(queryOptions.query) : queryOptions.query,
           query.variables || {},
           this.reducerConfig,
         );


Test patch:
diff --git a/test/mutationResults.ts b/test/mutationResults.ts
--- a/test/mutationResults.ts
+++ b/test/mutationResults.ts
@@ -13,33 +13,58 @@ import { ObservableQuery } from '../src/core/ObservableQuery';
 import gql from 'graphql-tag';
 
 describe('mutation results', () => {
+
   const query = gql`
     query todoList {
-      __typename
       todoList(id: 5) {
-        __typename
         id
         todos {
           id
-          __typename
           text
           completed
         }
         filteredTodos: todos(completed: true) {
           id
-          __typename
           text
           completed
         }
       }
       noIdList: todoList(id: 6) {
-        __typename
         id
         todos {
+          text
+          completed
+        }
+      }
+    }
+  `;
+
+  const queryWithTypename = gql`
+    query todoList {
+      todoList(id: 5) {
+        id
+        todos {
+          id
+          text
+          completed
           __typename
+        }
+        filteredTodos: todos(completed: true) {
+          id
           text
           completed
+          __typename
         }
+        __typename
+      }
+      noIdList: todoList(id: 6) {
+        id
+        todos {
+          text
+          completed
+          __typename
+        }
+        __typename
       }
     }
   `;
@@ -192,7 +217,7 @@ describe('mutation results', () => {
 
   function setupObsHandle(...mockedResponses: any[]) {
     networkInterface = mockNetworkInterface({
-      request: { query },
+      request: { query: queryWithTypename },
       result,
     }, ...mockedResponses);
 
@@ -215,7 +240,7 @@ describe('mutation results', () => {
 
   function setupDelayObsHandle(delay: number, ...mockedResponses: any[]) {
     networkInterface = mockNetworkInterface({
-      request: { query },
+      request: { query: queryWithTypename },
       result,
       delay,
     }, ...mockedResponses);
@@ -394,6 +419,7 @@ describe('mutation results', () => {
       }, {
         request: { query: mutation},
         result: mutationResult,
+        delay: 5,
       }, {
         request: { query: queryWithVars, variables: { id: 6 } },
         result: result6,
@@ -439,7 +465,7 @@ describe('mutation results', () => {
         subscription.unsubscribe();
 
         // The reducer should have been called twice
-        assert.equal(counter, 3);
+        assert.equal(counter, 4);
 
         // But there should be one more todo item than before, because variables only matched once
         assert.equal(newResult.data.todoList.todos.length, 4);
@@ -688,7 +714,7 @@ describe('mutation results', () => {
       // The resolver doesn't actually run.
       function setupReducerObsHandle(...mockedResponses: any[]) {
         networkInterface = mockNetworkInterface({
-          request: { query },
+          request: { query: queryWithTypename },
           result,
           delay: 30,
         }, ...mockedResponses);
@@ -750,7 +776,7 @@ describe('mutation results', () => {
       it('does not swallow errors', done => {
         client = new ApolloClient({
           networkInterface: mockNetworkInterface({
-            request: { query },
+            request: { query: queryWithTypename },
             result,
           }),
         });
@@ -984,7 +1010,7 @@ describe('mutation results', () => {
         request: { query: mutation },
         result: {errors: [new Error('mock error')]},
       }, {
-        request: { query },
+        request: { query: queryWithTypename },
         result,
       });
 
@@ -1435,7 +1461,7 @@ describe('mutation results', () => {
         request: { query: mutation },
         result: {errors: [new Error('mock error')]},
       }, {
-        request: { query },
+        request: { query: queryWithTypename },
         result,
       });