Query: Fix data is undefined on error

Changelog.md

@@ -13,8 +13,17 @@
 - The `graphql` `options` object is no longer mutated, when calculating
   variables from props. This now prevents an issue where components created
   with `graphql` were not having their query variables updated properly, when
-  props changed.
+  props changed. <br/>
   [@ksmth](https://github.com/ksmth) in [#1968](https://github.com/apollographql/react-apollo/pull/1968)
+- When a query failed on the first result, the query result `data` was being
+  returned as `undefined`. This behavior has been changed so that `data` is
+  returned as an empty object. This makes checking for data (e.g.
+  instead of `data && data.user` you can just check `data.user`) and
+  destructring (e.g. `{ data: { user } }`) easier. **Note:** this could
+  potentially hurt applications that are relying on a falsey check of `data`
+  to see if any query errors have occurred. A better (and supported) way to
+  check for errors is to use the result `errors` property. <br/>
+  [@TLadd](https://github.com/TLadd) in [#1983](https://github.com/apollographql/react-apollo/pull/1983)
 
 ## 2.1.11 (August 9, 2018)
 
@@ -27,13 +36,13 @@
   in `refetchQueries` will be completed before the mutation itself is
   completed. `awaitRefetchQueries` is `false` by default, which means
   `refetchQueries` are usually completed after the mutation has resolved.
-  Relates to Apollo Client
+  Relates to Apollo Client. <br/>
   [PR #3169](https://github.com/apollographql/apollo-client/pull/3169). <br/>
   [@hwillson](https://github.com/hwillson) in [#2214](https://github.com/apollographql/react-apollo/pull/2214)
 - Typings adjustment: pass `TData` along into `MutationUpdaterFn` when using
   `MutationOpts`, to ensure that the updater function is properly typed. <br/>
   [@danilobuerger](https://github.com/danilobuerger) in [#2227](https://github.com/apollographql/react-apollo/pull/2227)
-- Check if queryManager is set before accessing it.
+- Check if queryManager is set before accessing it. <br/>
   [@danilobuerger](https://github.com/danilobuerger) in [#2165](https://github.com/apollographql/react-apollo/pull/2165)
 
 ## 2.1.9 (July 4, 2018)

src/Query.tsx

@@ -94,7 +94,7 @@ export interface QueryResult<TData = any, TVariables = OperationVariables>
   // I'm aware of. So intead we enforce checking for data
   // like so result.data!.user. This tells TS to use TData
   // XXX is there a better way to do this?
-  data: TData | undefined;
+  data: TData | {};
   error?: ApolloError;
   loading: boolean;
   networkStatus: NetworkStatus;
@@ -391,9 +391,12 @@ export default class Query<TData = any, TVariables = OperationVariables> extends
     if (loading) {
       Object.assign(data.data, this.previousData, currentResult.data);
     } else if (error) {
-      Object.assign(data, {
-        data: (this.queryObservable!.getLastResult() || {}).data,
-      });
+      const lastResult = this.queryObservable!.getLastResult();
+      if (lastResult) {
+        Object.assign(data, {
+          data: lastResult.data,
+        });
+      }
     } else {
       Object.assign(data.data, currentResult.data);
       this.previousData = currentResult.data;

src/Subscriptions.tsx

@@ -144,8 +144,11 @@ class Subscription<TData = any, TVariables = any> extends React.Component<
   });
 
   private updateCurrentData = (result: SubscriptionResult<TData>) => {
-    const {client, props: {onSubscriptionData}} = this;
-    if (onSubscriptionData) onSubscriptionData({client, subscriptionData: result});
+    const {
+      client,
+      props: { onSubscriptionData },
+    } = this;
+    if (onSubscriptionData) onSubscriptionData({ client, subscriptionData: result });
     this.setState({
       data: result.data,
       loading: false,

test/client/Query.test.tsx

@@ -163,6 +163,7 @@ describe('Query component', () => {
               return null;
             }
             catchAsyncError(done, () => {
+              expect(result.data).toEqual({});
               expect(result.error).toEqual(new Error('Network error: error occurred'));
               done();
             });
@@ -1233,7 +1234,7 @@ describe('Query component', () => {
     function Container() {
       return (
         <AllPeopleQuery2 query={query} notifyOnNetworkStatusChange>
-          {result => {
+          {(result: any) => {
             try {
               switch (count++) {
                 case 0:

test/client/Subscription.test.tsx

@@ -98,7 +98,6 @@ it('executes the subscription', done => {
   jest.runTimersToTime(40);
 });
 
-
 it('calls onSubscriptionData if given', done => {
   jest.useFakeTimers();
 
@@ -109,7 +108,7 @@ it('calls onSubscriptionData if given', done => {
       subscription={subscription}
       onSubscriptionData={opts => {
         expect(opts.client).toBeInstanceOf(ApolloClient);
-        const {data} = opts.subscriptionData;
+        const { data } = opts.subscriptionData;
         expect(data).toEqual(results[count].result.data);
         if (count === 3) done();
         count++;

test/client/getDataFromTree.test.tsx

@@ -82,7 +82,11 @@ describe('SSR', () => {
       it('functional stateless components', () => {
         let elementCount = 0;
         const MyComponent = ({ n }: { n: number }) => (
-          <div>{_.times(n, i => <span key={i} />)}</div>
+          <div>
+            {_.times(n, i => (
+              <span key={i} />
+            ))}
+          </div>
         );
         walkTree(<MyComponent n={5} />, {}, () => {
           elementCount += 1;
@@ -99,7 +103,9 @@ describe('SSR', () => {
         }
         const MyComponent = ({ n, children }: Props) => (
           <div>
-            {_.times(n, i => <span key={i} />)}
+            {_.times(n, i => (
+              <span key={i} />
+            ))}
             {children}
           </div>
         );
@@ -129,7 +135,9 @@ describe('SSR', () => {
         let elementCount = 0;
         const MyComponent = ({ n, children = null }: { n: number; children: React.ReactNode }) => (
           <div>
-            {_.times(n, i => <span key={i} />)}
+            {_.times(n, i => (
+              <span key={i} />
+            ))}
             {children}
           </div>
         );
@@ -228,7 +236,13 @@ describe('SSR', () => {
         let elementCount = 0;
         class MyComponent extends React.Component<any, any> {
           render() {
-            return <div>{_.times(this.props.n, i => <span key={i} />)}</div>;
+            return (
+              <div>
+                {_.times(this.props.n, i => (
+                  <span key={i} />
+                ))}
+              </div>
+            );
           }
         }
         walkTree(<MyComponent n={5} />, {}, () => {
@@ -284,7 +298,13 @@ describe('SSR', () => {
             super(null); // note doesn't pass props or context
           }
           render() {
-            return <div>{_.times(this.props.n, i => <span key={i} />)}</div>;
+            return (
+              <div>
+                {_.times(this.props.n, i => (
+                  <span key={i} />
+                ))}
+              </div>
+            );
           }
         }
         walkTree(<MyComponent n={5} />, {}, () => {
@@ -299,7 +319,9 @@ describe('SSR', () => {
           render() {
             return (
               <div>
-                {_.times(this.props.n, i => <span key={i} />)}
+                {_.times(this.props.n, i => (
+                  <span key={i} />
+                ))}
                 {this.props.children}
               </div>
             );
@@ -321,7 +343,13 @@ describe('SSR', () => {
         let elementCount = 0;
         class MyComponent extends (React.Component as any) {
           render = () => {
-            return <div>{_.times(this.props.n, i => <span key={i} />)}</div>;
+            return (
+              <div>
+                {_.times(this.props.n, i => (
+                  <span key={i} />
+                ))}
+              </div>
+            );
           };
         }
         const MyCompAsAny = MyComponent as any;
@@ -503,17 +531,19 @@ describe('SSR', () => {
       });
 
       interface Data {
-        currentUser: {
+        currentUser?: {
           firstName: string;
         };
       }
 
       class CurrentUserQuery extends Query<Data> {}
 
+      const hasOwn = Object.prototype.hasOwnProperty;
+
       const WrappedElement = () => (
         <CurrentUserQuery query={query}>
-          {({ data, loading }) => (
-            <div>{loading || !data ? 'loading' : data.currentUser.firstName}</div>
+          {({ data, loading }: { data: Data; loading: boolean }) => (
+            <div>{loading || !data ? 'loading' : data.currentUser!.firstName}</div>
           )}
         </CurrentUserQuery>
       );
@@ -1252,7 +1282,7 @@ describe('SSR', () => {
       });
 
       interface Data {
-        currentUser: {
+        currentUser?: {
           firstName: string;
         };
       }
@@ -1261,8 +1291,8 @@ describe('SSR', () => {
 
       const Element = (props: { id: string }) => (
         <CurrentUserQuery query={query} ssr={false} variables={props}>
-          {({ data, loading }) => (
-            <div>{loading || !data ? 'loading' : data.currentUser.firstName}</div>
+          {({ data, loading }: { data: Data; loading: boolean }) => (
+            <div>{loading || !data ? 'loading' : data.currentUser!.firstName}</div>
           )}
         </CurrentUserQuery>
       );