Change the default query data state from {} to undefined

React Apollo returns an empty object for the initial `data` value,
that is returned when performing a query. There a long history
behind why this is the case, but this single issue has caused a
lot of frustration over the past long while due to the following:

- `{}` is not the same as having no data; it's an object, not
  nothing. If there is no data, we shouldn't pretend there is.
- Setting `data` to `{}` isn't followed in all cases; e.g. when
  errors occur, during parts of SSR, when skipping, etc. This leads
  to developers having to know when to expect `{}` and when to
  expect `undefined`.
- Forcing no data situations to be empty objects can go against
  application schemas that enforce a value and don't allow
  empty objects.

This commit adjusts React Apollo's default query `data` state to
be `undefined`, and will no longer convert `data` to an empty
object if the returned data is falsy. Using `undefined` as the
initial state has also been aligned across all parts of RA
querying, including SSR.

Unfortunately, we missed the opportunity to address this as part
of the React Apollo 3.0 launch. This change can be considered
breaking, and we can't do another major version bump to get this
in (3.0 is the last major version of React Apollo, as React
Apollo is being merged into the Apollo Client project). We'll
have to put some thought into how we can roll this out.

Fixes #3323.

Changelog.md

@@ -1,5 +1,16 @@
 # Change log
 
+## 3.1.0 (TBD)
+
+### Improvements
+
+- Change the default query `data` state from `{}` to `undefined`.
+
+  > **NOTE: THIS IS A POTENTIALLY BREAKING CHANGE.** We're still working out the best way to roll this out.
+
+  This change aligns all parts of the React Apollo query cycle so that `data` is always `undefined` if there is no data, instead of `data` being converted into an empty object. This change impacts the initial query response, initial SSR response, `data` value when errors occur, `data` value when skipping, etc. All of these areas are now aligned to only ever return a value for `data` if there really is a value to return (instead of making it seem like there is one by converting to `{}`). <br/>
+  [@hwillson](https://github.com/hwillson) in [#3388](https://github.com/apollographql/react-apollo/pull/3388)
+
 ## 3.0.1 (2019-08-15)
 
 ### Improvements

packages/components/src/__tests__/client/Query.test.tsx

@@ -9,7 +9,7 @@ import {
 } from '@apollo/react-testing';
 import { DocumentNode } from 'graphql';
 import gql from 'graphql-tag';
-import { render, cleanup } from '@testing-library/react';
+import { render, cleanup, wait } from '@testing-library/react';
 import { ApolloLink } from 'apollo-link';
 import { Query } from '@apollo/react-components';
 
@@ -954,7 +954,7 @@ describe('Query component', () => {
       );
     });
 
-    it('onError with error', done => {
+    it('onError with error', async () => {
       const error = new Error('error occurred');
       const mockError = [
         {
@@ -965,7 +965,6 @@ describe('Query component', () => {
 
       const onErrorFunc = (queryError: ApolloError) => {
         expect(queryError.networkError).toEqual(error);
-        done();
       };
 
       const Component = () => (
@@ -981,6 +980,8 @@ describe('Query component', () => {
           <Component />
         </MockedProvider>
       );
+
+      await wait();
     });
   });
 
@@ -1958,7 +1959,7 @@ describe('Query component', () => {
             {(result: any) => {
               const { data, loading } = result;
               if (!loading) {
-                expect(data).toEqual({});
+                expect(data).toBeUndefined();
                 done();
               }
               return null;
@@ -2133,7 +2134,7 @@ describe('Query component', () => {
         <ApolloProvider client={client}>
           <Query query={partialQuery}>
             {({ data }: any) => {
-              expect(data).toEqual({});
+              expect(data).toBeUndefined();
               return null;
             }}
           </Query>

packages/components/src/__tests__/client/__snapshots__/Query.test.tsx.snap

@@ -21,7 +21,7 @@ Object {
 exports[`Query component calls the children prop: result in render prop while loading 1`] = `
 Object {
   "called": true,
-  "data": Object {},
+  "data": undefined,
   "error": undefined,
   "fetchMore": [Function],
   "loading": true,

packages/hooks/src/__tests__/useQuery.test.tsx

@@ -58,6 +58,25 @@ describe('useQuery Hook', () => {
         </MockedProvider>
       );
     });
+
+    it('should keep data as undefined until data is actually returned', done => {
+      const Component = () => {
+        const { data, loading } = useQuery(CAR_QUERY);
+        if (loading) {
+          expect(data).toBeUndefined();
+        } else {
+          expect(data).toEqual(CAR_RESULT_DATA);
+          done();
+        }
+        return null;
+      };
+
+      render(
+        <MockedProvider mocks={CAR_MOCKS}>
+          <Component />
+        </MockedProvider>
+      );
+    });
   });
 
   describe('Polling', () => {

packages/hooks/src/data/QueryData.ts

@@ -173,7 +173,7 @@ export class QueryData<TData, TVariables> extends OperationData {
       loading: true,
       networkStatus: NetworkStatus.loading,
       called: true,
-      data: {}
+      data: undefined
     };
 
     if (this.context && this.context.renderPromises) {
@@ -275,7 +275,7 @@ export class QueryData<TData, TVariables> extends OperationData {
           this.previousData.result &&
           this.previousData.result.loading === loading &&
           this.previousData.result.networkStatus === networkStatus &&
-          isEqual(this.previousData.result.data, data || {})
+          isEqual(this.previousData.result.data, data)
         ) {
           return;
         }
@@ -314,15 +314,9 @@ export class QueryData<TData, TVariables> extends OperationData {
   }
 
   private getQueryResult(): QueryResult<TData, TVariables> {
-    let result = {
-      data: Object.create(null) as TData
-    } as any;
-
-    // Attach bound methods
-    Object.assign(
-      result,
-      this.observableQueryFields(this.currentObservable.query!)
-    );
+    let result: any = {
+      ...this.observableQueryFields(this.currentObservable.query!)
+    };
 
     // When skipping a query (ie. we're not querying for data but still want
     // to render children), make sure the `data` is cleared out and
@@ -340,21 +334,31 @@ export class QueryData<TData, TVariables> extends OperationData {
       const currentResult = this.currentObservable.query!.getCurrentResult();
       const { loading, partial, networkStatus, errors } = currentResult;
       let { error, data } = currentResult;
-      data = data || (Object.create(null) as TData);
 
       // Until a set naming convention for networkError and graphQLErrors is
       // decided upon, we map errors (graphQLErrors) to the error options.
       if (errors && errors.length > 0) {
         error = new ApolloError({ graphQLErrors: errors });
       }
 
-      Object.assign(result, { loading, networkStatus, error, called: true });
+      result = {
+        ...result,
+        loading,
+        networkStatus,
+        error,
+        called: true
+      };
 
       if (loading) {
-        const previousData = this.previousData.result
-          ? this.previousData.result.data
-          : {};
-        Object.assign(result.data, previousData, data);
+        const previousData =
+          this.previousData.result && this.previousData.result.data;
+        result.data =
+          previousData && data
+            ? {
+                ...previousData,
+                ...data
+              }
+            : previousData || data;
       } else if (error) {
         Object.assign(result, {
           data: (this.currentObservable.query!.getLastResult() || ({} as any))
@@ -365,14 +369,14 @@ export class QueryData<TData, TVariables> extends OperationData {
         const { partialRefetch } = this.getOptions();
         if (
           partialRefetch &&
-          Object.keys(data).length === 0 &&
+          !data &&
           partial &&
           fetchPolicy !== 'cache-only'
         ) {
           // When a `Query` component is mounted, and a mutation is executed
           // that returns the same ID as the mounted `Query`, but has less
           // fields in its result, Apollo Client's `QueryManager` returns the
-          // data as an empty Object since a hit can't be found in the cache.
+          // data as `undefined` since a hit can't be found in the cache.
           // This can lead to application errors when the UI elements rendered by
           // the original `Query` component are expecting certain data values to
           // exist, and they're all of a sudden stripped away. To help avoid
@@ -385,7 +389,7 @@ export class QueryData<TData, TVariables> extends OperationData {
           return result;
         }
 
-        Object.assign(result.data, data);
+        result.data = data;
       }
     }
 
@@ -417,7 +421,7 @@ export class QueryData<TData, TVariables> extends OperationData {
       }
 
       if (onCompleted && !error) {
-        onCompleted(data as TData);
+        onCompleted(data);
       } else if (onError && error) {
         onError(error);
       }

packages/ssr/src/__tests__/useQuery.test.tsx

@@ -62,11 +62,11 @@ describe('useQuery Hook SSR', () => {
     });
   });
 
-  it('should initialize data as an empty object when loading', () => {
+  it('should initialize data as `undefined` when loading', () => {
     const Component = () => {
       const { data, loading } = useQuery(CAR_QUERY);
       if (loading) {
-        expect(data).toEqual({});
+        expect(data).toBeUndefined();
       }
       return null;
     };