Repository: FormidableLabs/urql (ts)

Issue:
Cannot Page will Nullable Nested Objects:  TypeError: Cannot set property '__typename' of null
urql: 2.3.0 +
Exchanges: @urql/exchange-graphcache, fetch

**Steps to reproduce**

1. Setup a client with normalized cache and fetch.
2. Setup relayPaging for data that contains a nullable object type.
3. Fetch page 1
4. Fetch page 2 (fails)

I have a simple test, it will use `relayPagination` and fetch the first two pages.
My code is slightly anomyzed from the real prod code but looks like

```ts
import {
  createClient,
  fetchExchange,
  OperationContext,
  Exchange
} from "urql";
import {
  Query,
  QueryVariables,
  Document
} from "generated/graphql"; //Our generated query and variables
import {DocumentNode, IntrospectionQuery} from "graphql";
import {PgHeaders} from "./playgroundheaders";
import introspection from "generated/introspection.json";
import { cacheExchange } from "@urql/exchange-graphcache";
import {CacheExchangeOpts} from "@urql/exchange-graphcache/dist/types/cacheExchange";
import {relayPagination} from "@urql/exchange-graphcache/extras";
// @ts-ignore
const schema: IntrospectionQuery = introspection as IntrospectionQuery;

const cacheExchangeOpts: CacheExchangeOpts = {
  schema,
  keys: {
      //We have some intentionally null keys for embedded data
  },
  resolvers: {
    Query: {
      indicators: relayPagination({
        mergeMode: "outwards"
      })
    }
  }
};
const setupReal = () => {
  const url = "<!--our host -->"
  const exchanges: Array<Exchange> = [
    cacheExchange(cacheExchangeOpts),
    fetchExchange
  ]
  const client = createClient({
    url,
    requestPolicy: "network-only", //Force paging tests to wait for a response, the real will give partial results first.
    fetchOptions: {
      headers: {
        ...PgHeaders //throw our auth headers on
      }
    },
    fetch: (input, init) => {
      console.log("Request");
      return fetch(input, init).then(reso => {
        console.log("answered");
        return reso;
      });
    },
    exchanges
  })
  //Query helper alias for test
  function query<Q = any, V extends object = {}>(query: DocumentNode | string, variables?: V, context?: Partial<OperationContext>) {
    return client.query<Q, V>(query, variables, context).toPromise()
  }
  return {client, query}
}
test("Simple test", async () => {
  const { query } = setupReal()
  const variables: QueryVariables = {
    //Setup variables
  }
  const res = await query<Query, QueryVariables>(Document, variables)
  //50 is our default page size.
  expect(res.data?.indicators?.edges.length).toBeGreaterThanOrEqual(50);
  //Nab page 2
  const after = res?.data?.indicators?.pageInfo.endCursor;
  const res2 = await query<Query, QueryVariables>(Document, {
    ...variables,
    after
  }) //ERROR HAPPENS HERE
  expect(res2.data?.indicators?.edges.length).toBeGreaterThan(51);
}, 20000)
```


The query is  like

```graphql
query testQuery($input: InputType!, $after: String) {
  indicators(input: $input, after: $after) {
    count
    pageInfo {  
        endCursor
        hasNextPage
    }
    edges {
      cursor
      node {
        ...staticFields
        nullableObject {
            prop1
        }
      }
    }
  }
}
```

Type definition like

```
Query{
  indicators(input: InputType, after: String, first: Int): IndicatorConnection!
}
type IndicatorConnection {
  edges: [IndicatorEdge!]!
  pageInfo: PageInfo!
  count: Int!
}
type IndicatorEdge {
  cursor: Cursor!
  node: Indicator!
}
type Indicator {
  id: ID!
  nullableObject: NullableObject
}
```


Output in 2.2.2 (test passes)
```
/home/gsporie/.nvm/versions/node/v12.8.1/bin/node --require /snap/intellij-idea-ultimate/217/plugins/JavaScriptLanguage/helpers/jest-intellij/lib/jest-intellij-stdin-fix.js /home/gsporie/IdeaProjects/glass-review/node_modules/react-scripts/bin/react-scripts.js test --colors --reporters /snap/intellij-idea-ultimate/217/plugins/JavaScriptLanguage/helpers/jest-intellij/lib/jest-intellij-reporter.js --verbose --runTestsByPath /home/gsporie/IdeaProjects/glass-review/src/polygraphql/__integration__/graphqlClient.test.ts
  console.log src/polygraphql/__integration__/graphqlClient.test.ts:52
    Request

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:54
    answered

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:52
    Request

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:54
    answered
```

Output in 2.3.0 (fails)

```
  console.log src/polygraphql/__integration__/graphqlClient.test.ts:52
    Request

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:54
    answered

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:52
    Request

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:54
    answered


TypeError: Cannot set property '__typename' of null

    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:671:18)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:469:12)
    at resolveLink (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:339:26)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:469:12)
    at resolveLink (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:339:26)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:421:9)
    at resolveResolverResult (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:397:27)
    at resolveResolverResult (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:325:24)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:422:9)
    at resolveResolverResult (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:300:26)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:95:9)
    at read (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/operations/query.ts:65:18)
    at query (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/src/cacheExchange.ts:239:27)
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:759:56
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:492:42
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:492:42
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:1027:26
    at next (/home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:108:24)
    at next (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/core/src/exchanges/fetch.ts:125:23)
```


Output 2.4.0

```
  console.log src/polygraphql/__integration__/graphqlClient.test.ts:52
    Request

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:54
    answered

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:52
    Request

  console.log src/polygraphql/__integration__/graphqlClient.test.ts:54
    answered


TypeError: Cannot set property '__typename' of null

    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:710:20)
    at resolveLink (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:775:30)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:726:169)
    at resolveLink (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:775:30)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:726:169)
    at resolveResolverResult (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:759:62)
    at resolveResolverResult (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:747:27)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:726:66)
    at resolveResolverResult (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:759:90)
    at readSelection (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:721:45)
    at read (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:655:63)
    at query (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:647:7)
    at C (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:937:67)
    at g (/home/gsporie/IdeaProjects/glass-review/node_modules/@urql/exchange-graphcache/dist/urql-exchange-graphcache.js:828:28)
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:759:56
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:748:28
    at b (/home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:865:14)
    at Array.forEach (<anonymous>)
    at b (/home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:864:77)
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:492:42
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:492:42
    at /home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:1027:26
    at next (/home/gsporie/IdeaProjects/glass-review/node_modules/wonka/dist/wonka.js:108:24)
    at next (/home/gsporie/IdeaProjects/glass-review/node_modules/urql/node_modules/@urql/core/src/exchanges/fetch.ts:125:23)
```

**Expected behavior**

I expect it to page successfully.

This test will pass in 2.2.2, but fails in 2.3.0+


**Actual behavior**

A type error is throw from the cache exchange.

Pull request description:
(graphache) - Fix query traversal for previous null fields
Fix #768

## Summary

This issue occurs when a previous selection determined a field to be `null`. A future selection that doesn't result in `null` is then forced to retrieved `prevData`, but isn't covering the `null` case.

This can happen with split selections on a single entity, e.g.

```
query Item {
  ...NullItem
  ...ValidItem
}

fragment NullItem {
  item {
    id
    invalid # Suppose this causes the item to become null, due to an invalid field
  }
}

fragment ValidItem {
  item {
    id
  }
}
```

- On the first traversal, the `NullItem` fragment hits `invalid` which causes `Query.item` to become `null`
- On the second traversal, the `Validitem` fragment hits `id` and tries to write it, but `Query.item` is already `null`

This can only happen with schema awareness most of the time.

## Set of changes

- Handle `prevData === null` by switching most of its uses to a truthy rather than an `undefined` check.
- Fix case where `prevData === null` leads to further traversal

## Outstanding edge cases

~The **read** edge case can be seen in this PR's test case, when a traversal fails and returns `undefined`, then the second traversal doesn't set the affected field to `null`.~

Patch:
diff --git a/.changeset/slimy-chairs-sniff.md b/.changeset/slimy-chairs-sniff.md
new file mode 100644
index 00000000..b87085b2
--- /dev/null
+++ b/.changeset/slimy-chairs-sniff.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-graphcache': patch
+---
+
+Fix traversal issue, where when a prior selection set has set a nested result field to `null`, a subsequent traversal of this field attempts to access `prevData` on `null`.
diff --git a/exchanges/graphcache/src/operations/query.ts b/exchanges/graphcache/src/operations/query.ts
index 1582b5ce..3aa56df7 100644
--- a/exchanges/graphcache/src/operations/query.ts
+++ b/exchanges/graphcache/src/operations/query.ts
@@ -391,7 +391,7 @@ const resolveResolverResult = (
   fieldName: string,
   key: string,
   select: SelectionSet,
-  prevData: void | Data | Data[],
+  prevData: void | null | Data | Data[],
   result: void | DataField
 ): DataField | void => {
   if (Array.isArray(result)) {
@@ -410,7 +410,7 @@ const resolveResolverResult = (
         joinKeys(key, `${i}`),
         select,
         // Get the inner previous data from prevData
-        prevData !== undefined ? prevData[i] : undefined,
+        prevData != null ? prevData[i] : undefined,
         result[i]
       );
 
@@ -424,8 +424,12 @@ const resolveResolverResult = (
     return data;
   } else if (result === null || result === undefined) {
     return result;
+  } else if (prevData === null) {
+    // If we've previously set this piece of data to be null,
+    // we skip it and return null immediately
+    return null;
   } else if (isDataOrKey(result)) {
-    const data = (prevData === undefined ? {} : prevData) as Data;
+    const data = (prevData || {}) as Data;
     return typeof result === 'string'
       ? readSelection(ctx, result, select, data)
       : readSelection(ctx, key, select, data, result);
@@ -448,7 +452,7 @@ const resolveLink = (
   typename: string,
   fieldName: string,
   select: SelectionSet,
-  prevData: void | Data | Data[]
+  prevData: void | null | Data | Data[]
 ): DataField | undefined => {
   if (Array.isArray(link)) {
     const { store } = ctx;
@@ -462,7 +466,7 @@ const resolveLink = (
         typename,
         fieldName,
         select,
-        prevData !== undefined ? prevData[i] : undefined
+        prevData != null ? prevData[i] : undefined
       );
       if (childLink === undefined && !_isListNullable) {
         return undefined;
@@ -472,15 +476,12 @@ const resolveLink = (
     }
 
     return newLink;
-  } else if (link === null) {
+  } else if (link === null || prevData === null) {
+    // If the link is set to null or we previously set this piece of data to be null,
+    // we skip it and return null immediately
     return null;
   } else {
-    return readSelection(
-      ctx,
-      link,
-      select,
-      prevData === undefined ? ({} as any) : prevData
-    );
+    return readSelection(ctx, link, select, (prevData || {}) as Data);
   }
 };
 
diff --git a/packages/react-urql/examples/1-getting-started/package.json b/packages/react-urql/examples/1-getting-started/package.json
index 7e42ea66..ebaf8f96 100644
--- a/packages/react-urql/examples/1-getting-started/package.json
+++ b/packages/react-urql/examples/1-getting-started/package.json
@@ -4,7 +4,8 @@
   "main": "src/index.html",
   "license": "MIT",
   "scripts": {
-    "start": "parcel src/index.html"
+    "start": "parcel src/index.html",
+    "build": "parcel build src/index.html"
   },
   "devDependencies": {
     "@types/react": "^16.9.23",


Test patch:
diff --git a/exchanges/graphcache/src/operations/query.test.ts b/exchanges/graphcache/src/operations/query.test.ts
index a7adb78b..3f819546 100644
--- a/exchanges/graphcache/src/operations/query.test.ts
+++ b/exchanges/graphcache/src/operations/query.test.ts
@@ -187,4 +187,65 @@ describe('Query', () => {
       todos: [{ __typename: 'Todo', id: '0', text: 'Solve bug' }],
     });
   });
+
+  it('should allow subsequent read when first result was null', () => {
+    const QUERY_WRITE = gql`
+      query writeTodos {
+        todos {
+          ...ValidRead
+        }
+      }
+
+      fragment ValidRead on Todo {
+        id
+      }
+    `;
+
+    const QUERY_READ = gql`
+      query getTodos {
+        todos {
+          ...MissingRead
+        }
+        todos {
+          id
+        }
+      }
+
+      fragment ValidRead on Todo {
+        id
+      }
+
+      fragment MissingRead on Todo {
+        id
+        text
+      }
+    `;
+
+    const store = new Store({
+      schema: alteredRoot,
+    });
+
+    let { data } = query(store, { query: QUERY_READ });
+    expect(data).toEqual(null);
+
+    write(
+      store,
+      { query: QUERY_WRITE },
+      {
+        todos: [
+          {
+            __typename: 'Todo',
+            id: '0',
+          },
+        ],
+        __typename: 'Query',
+      }
+    );
+
+    ({ data } = query(store, { query: QUERY_READ }));
+    expect(data).toEqual({
+      __typename: 'query_root',
+      todos: [null],
+    });
+  });
 });