Fix skip causing loading to always be true during SSR (#7567)

PR #7310 introduced a regression that caused skipped SSR queries to
always set `loading` to `true`, which doesn't match the behavior of
`useQuery()` on the client and can result in hydration mismatches.

The problem is that `skip: true` was being treated as equivalent to
`ssr: false`, but they're not actually equivalent. I think the correct
solution is to let `this.getQueryResult()` provide a suitable result
when `skip` is truthy, which ensures that skipped SSR queries will get
the same result as skipped non-SSR queries.

Fixes #7380

Co-authored-by: Ben Newman <ben@apollographql.com>

CHANGELOG.md

@@ -1,3 +1,10 @@
+## Apollo Client 3.3.7
+
+### Bug Fixes
+
+- Fix a regression due to [#7310](https://github.com/apollographql/apollo-client/pull/7310) that caused `loading` always to be `true` for `skip: true` results during server-side rendering. <br/>
+  [@rgrove](https://github.com/rgrove) in [#7567](https://github.com/apollographql/apollo-client/pull/7567)
+
 ## Apollo Client 3.3.6
 
 ### Bug Fixes

src/react/data/OperationData.ts

@@ -12,7 +12,7 @@ export abstract class OperationData<TOptions = any> {
     TOptions
   >;
   public context: any = {};
-  public client: ApolloClient<object> | undefined;
+  public client: ApolloClient<object>;
 
   private options: CommonOptions<TOptions> = {} as CommonOptions<TOptions>;
 

src/react/data/QueryData.ts

@@ -153,7 +153,7 @@ export class QueryData<TData, TVariables> extends OperationData {
 
   private getExecuteSsrResult() {
     const { ssr, skip } = this.getOptions();
-    const ssrDisabled = ssr === false || skip;
+    const ssrDisabled = ssr === false;
     const fetchDisabled = this.refreshClient().client.disableNetworkFetches;
 
     const ssrLoading = {
@@ -175,11 +175,15 @@ export class QueryData<TData, TVariables> extends OperationData {
 
     let result;
     if (this.ssrInitiated()) {
-      result =
-        this.context.renderPromises!.addQueryPromise(
-          this,
-          this.getQueryResult
-        ) || ssrLoading;
+      if (skip) {
+        result = this.getQueryResult();
+      } else {
+        result =
+          this.context.renderPromises!.addQueryPromise(
+            this,
+            this.getQueryResult
+          ) || ssrLoading;
+      };
     }
 
     return result;
@@ -333,7 +337,7 @@ export class QueryData<TData, TVariables> extends OperationData {
   }
 
   private getQueryResult = (): QueryResult<TData, TVariables> => {
-    let result: any = this.observableQueryFields();
+    let result = this.observableQueryFields() as QueryResult<TData, TVariables>;
     const options = this.getOptions();
 
     // When skipping a query (ie. we're not querying for data but still want
@@ -352,7 +356,8 @@ export class QueryData<TData, TVariables> extends OperationData {
         data: undefined,
         error: undefined,
         loading: false,
-        called: true
+        networkStatus: NetworkStatus.ready,
+        called: true,
       };
     } else if (this.currentObservable) {
       // Fetch the current result (if any) from the store.

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

@@ -170,13 +170,17 @@ describe('useQuery Hook SSR', () => {
   it('should skip SSR tree rendering if `skip` option is `true`', async () => {
     let renderCount = 0;
     const Component = () => {
-      const { data, loading } = useQuery(CAR_QUERY, { skip: true });
+      const {
+        loading,
+        networkStatus,
+        data,
+      } = useQuery(CAR_QUERY, { skip: true });
       renderCount += 1;
 
-      if (!loading) {
-        const { make } = data.cars[0];
-        return <div>{make}</div>;
-      }
+      expect(loading).toBeFalsy();
+      expect(networkStatus).toBe(7);
+      expect(data).toBeUndefined();
+
       return null;
     };
 
@@ -186,9 +190,9 @@ describe('useQuery Hook SSR', () => {
       </MockedProvider>
     );
 
-    return renderToStringWithData(app).then((result) => {
+    return renderToStringWithData(app).then(result => {
       expect(renderCount).toBe(1);
-      expect(result).toEqual('');
+      expect(result).toBe('');
     });
   });
 });