Repository: apollographql/apollo-client

Problem statement:
Getting "Can't find field" errors on null fields (even if the field is nullable)
Since 0.4.16.

Once a mutation comes back and its results are reinjected into the store via `updateQueries`

```
Can't find field cursor on result object $ROOT_QUERY.connection({"id":"xxx"}).nodeChildren({"id":"root","limit":40,"cursor":null,"sortBy":null}).
Error
    at new ApolloError (webpack:///./~/apollo-client/lib/src/errors.js?:14:22)
    at eval (webpack:///./~/apollo-client/lib/src/data/writeToStore.js?:59:23)
    at Array.forEach (native)
    at writeSelectionSetToStore (webpack:///./~/apollo-client/lib/src/data/writeToStore.js?:53:29)
    at eval (webpack:///./~/apollo-client/lib/src/data/writeToStore.js?:121:21)
    at Array.forEach (native)
    at writeSelectionSetToStore (webpack:///./~/apollo-client/lib/src/data/writeToStore.js?:53:29)
    at writeFieldToStore (webpack:///./~/apollo-client/lib/src/data/writeToStore.js?:226:9)
    at eval (webpack:///./~/apollo-client/lib/src/data/writeToStore.js?:75:17)
    at Array.forEach (native)
```



Hints:
This error should happen on `master` - can you submit a failing test as a PR?

Working on it 😉 !

Harder to reproduce than expected. Seems like I have to throw fragments in the mix!

OK, maybe it's a result of that deep-assign PR for fragments then? That's the only relevant change I can think of.

Urr, I'm based on an older version and can't get it to fail, trying with the **latest** master now!

But, yes, I think this PR, even if it'll help us **a lot** may be the one responsible

Okay, maybe I'm not in a good setting to write the test but I see how it's happening in our app: basically, the deepMerge seems to "transform" null fields into undefined.

Wow that would be really silly.

Yes, I agree, that's why I want to get the test first because it seems crazy

Okay I need to go for today, we'll freeze for now, I'll try to get the test ASAP, I think I'm not writing it at the right place: any advice on where I should write it ? For now I tried in `mutationResults` don't really see where to put it apart from there but I tried to make it fail there by adding an `undefined` (which we want to see fail) and didn't.

Tried it here: https://github.com/apollostack/apollo-client/compare/fix-678?expand=1

@rricard is that last link for a test that successfully reproduces the bug, or was it just an (unsuccessful) attempt?

Guys, I don't even think we need to write a fancy test for this, deep-assign doesn't copy null values:

https://github.com/sindresorhus/deep-assign/blob/699948936d466e2ea738610fcf7cc80aa21babf7/test.js#L13

```
test('do not assign null values', t => {
    t.deepEqual(fn({}, {foo: null}), {});
});
```

I'll find some other deep-copy thing we can use, or I'll write one myself if there's nothing decent out there.


Patch:
diff --git a/src/data/diffAgainstStore.ts b/src/data/diffAgainstStore.ts
--- a/src/data/diffAgainstStore.ts
+++ b/src/data/diffAgainstStore.ts
@@ -2,7 +2,7 @@ import isArray = require('lodash.isarray');
 import isNull = require('lodash.isnull');
 import isObject = require('lodash.isobject');
 import has = require('lodash.has');
-import deepAssign = require('deep-assign');
+import merge = require('lodash.merge');
 
 import {
   storeKeyNameFromField,
@@ -221,7 +221,7 @@ export function diffSelectionSetAgainstStore({
           }
 
           if (isObject(fieldResult)) {
-            deepAssign(result, fieldResult);
+            merge(result, fieldResult);
           }
           if (!fragmentErrors[typename]) {
             fragmentErrors[typename] = null;
@@ -260,7 +260,7 @@ export function diffSelectionSetAgainstStore({
             pushMissingField(selection);
           }
           if (isObject(fieldResult)) {
-            deepAssign(result, fieldResult);
+            merge(result, fieldResult);
           }
 
           if (!fragmentErrors[typename]) {


Test patch:
diff --git a/test/readFromStore.ts b/test/readFromStore.ts
--- a/test/readFromStore.ts
+++ b/test/readFromStore.ts
@@ -172,7 +172,9 @@ describe('reading from the store', () => {
       deepNestedObj: {
         stringField: 'This is a deep string',
         numberField: 7,
+        nullField: null,
       } as StoreObject,
+      nullObject: null,
     };
 
     const store = {
@@ -180,6 +182,7 @@ describe('reading from the store', () => {
         nestedObj: {
           type: 'id',
           id: 'abcde',
+          nullField: null,
           generated: false,
         },
       }) as StoreObject,
@@ -188,6 +191,7 @@ describe('reading from the store', () => {
           type: 'id',
           id: 'abcdef',
           generated: false,
+          nullField: null,
         },
       }) as StoreObject,
       abcdef: result.deepNestedObj as StoreObject,
@@ -199,22 +203,30 @@ describe('reading from the store', () => {
         fragment FragmentName on Item {
           stringField,
           numberField,
+          nullField,
           ...on Item {
             nestedObj {
               stringField
+              nullField
               deepNestedObj {
                 stringField
+                nullField
               }
             }
           }
           ...on Item {
             nestedObj {
               numberField
+              nullField
               deepNestedObj {
                 numberField
+                nullField
               }
             }
           }
+          ... on Item {
+            nullObject
+          }
         }
       `,
       rootId: 'abcd',
@@ -224,14 +236,18 @@ describe('reading from the store', () => {
     assert.deepEqual(queryResult, {
       stringField: 'This is a string!',
       numberField: 5,
+      nullField: null,
       nestedObj: {
         stringField: 'This is a string too!',
         numberField: 6,
+        nullField: null,
         deepNestedObj: {
           stringField: 'This is a deep string',
           numberField: 7,
+          nullField: null,
         },
       },
+      nullObject: null,
     });
   });