ssr false queries still being called on server (#3515)

* ssr false queries still being called on server * add test for this usecase * my added test was not adding any value, because the test is not testing the query being executed or not since that is happening in the startQuerySubscription part * Fully disable SSR when using `ssr: false` and `ssrMode: true` When `ssrMode` is `true`, Apollo Client's `disableNetworkFetches` is `true`, meaning we want all network fetches to be disabled. Right now `ssr: false` doesn't take `disableNetworkFetches` into consideration, but with these changes it will. So with these changes: - If `ssr: false` is set and we're coming from a React SSR `getDataFromTree` or `getMarkupFromTree` call (`treeRenderingInitiated` is `true`), we return a default SSR loading state to prevent the running of any subsequent queries in the same render. - If `ssr: false` is set and we've disabled network fetching (`ssrMode` is `true`), we return a default SSR loading state to prevent the running of any subsequent queries in the same render. * Prep for beta publish * Publish - react-apollo@3.1.2-beta.0 - @apollo/react-common@3.1.2-beta.0 - @apollo/react-components@3.1.2-beta.0 - @apollo/react-hoc@3.1.2-beta.0 - @apollo/react-hooks@3.1.2-beta.0 - @apollo/react-ssr@3.1.2-beta.0 - @apollo/react-testing@3.1.2-beta.0 * Changelog updates
apollographql · Oct 1, 2019 · 7b4fc9f · 7b4fc9f
1 parent b07971e
commit 7b4fc9f
Show file tree

Hide file tree

Showing 16 changed files with 141 additions and 67 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -1,5 +1,12 @@
 # Change log
 
+## 3.1.2 (not yet released)
+
+### Bug Fixes
+
+- Make sure SSR is fully disabled when using `ssr: false` and `ssrMode: true`. <br/>
+  [@maapteh](https://github.com/maapteh) in [#3515](https://github.com/apollographql/react-apollo/pull/3515)
+
 ## 3.1.1 (2019-09-15)
 
 ### Improvements

diff --git a/examples/ssr/hooks/package.json b/examples/ssr/hooks/package.json
@@ -8,6 +8,7 @@
   },
   "dependencies": {
     "@apollo/react-hooks": "3.1.1",
+    "@apollo/react-ssr": "^3.1.1",
     "@babel/runtime": "^7.4.5",
     "apollo-cache-inmemory": "^1.6.0",
     "apollo-client": "^2.6.0",

diff --git a/examples/ssr/hooks/server/main.js b/examples/ssr/hooks/server/main.js
@@ -1,7 +1,8 @@
 import { renderToString } from 'react-dom/server';
 import { onPageLoad } from 'meteor/server-render';
 import { ApolloClient } from 'apollo-client';
-import { getMarkupFromTree, ApolloProvider } from '@apollo/react-hooks';
+import { ApolloProvider } from '@apollo/react-hooks';
+import { getMarkupFromTree } from '@apollo/react-ssr';
 import { InMemoryCache } from 'apollo-cache-inmemory';
 import { HttpLink } from 'apollo-link-http';
 import { WebApp } from 'meteor/webapp';

diff --git a/package.json b/package.json
@@ -17,7 +17,7 @@
     "test:umd:ci": "npx jest --config ./config/jest.umd.config.js --ci --maxWorkers=2 --reporters=default --reporters=jest-junit --silent",
     "bundlesize": "npx lerna run build && bundlesize",
     "prettier": "npx prettier --config ./config/prettier.config.js --write \"./**/*.{js,jsx,ts*,md,graphql,json}\"",
-    "deploy": "npx lerna publish",
+    "deploy": "npx lerna publish --dist-tag beta",
     "clean": "rm -Rf ./node_modules ./meta && npx lerna exec -- npm run clean"
   },
   "dependencies": {

diff --git a/packages/all/package.json b/packages/all/package.json
@@ -1,7 +1,7 @@
 {
   "name": "react-apollo",
   "description": "React Apollo Hooks, Components, and HOC.",
-  "version": "3.1.1",
+  "version": "3.1.2-beta.0",
   "author": "opensource@apollographql.com",
   "keywords": [
     "apollo",
@@ -30,7 +30,7 @@
     "build": "npx tsc -p ./config",
     "postbuild": "npx rollup -c ./config/rollup.config.js",
     "predeploy": "npm run build",
-    "deploy": "npm publish"
+    "deploy": "npm publish --tag beta"
   },
   "peerDependencies": {
     "@types/react": "^16.8.0",

diff --git a/packages/common/package.json b/packages/common/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@apollo/react-common",
   "description": "React Apollo common utilities.",
-  "version": "3.1.1",
+  "version": "3.1.2-beta.0",
   "author": "opensource@apollographql.com",
   "keywords": [
     "apollo",
@@ -25,7 +25,7 @@
     "postbuild": "npx rollup -c ./config/rollup.config.js",
     "watch": "npx tsc-watch --onSuccess \"npm run postbuild\" -p ./config",
     "predeploy": "npm run build",
-    "deploy": "npm publish",
+    "deploy": "npm publish --tag beta",
     "test": "npx jest --config ../../config/jest.config.js --testPathPattern packages/common",
     "test:watch": "npx jest --config ../../config/jest.config.js --testPathPattern packages/common --watch",
     "test:cjs": "npm run build && npx jest --config ../../config/jest.cjs.config.js --testPathPattern packages/common",

diff --git a/packages/components/package.json b/packages/components/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@apollo/react-components",
   "description": "React Apollo Query, Mutation and Subscription components.",
-  "version": "3.1.1",
+  "version": "3.1.2-beta.0",
   "author": "opensource@apollographql.com",
   "keywords": [
     "apollo",
@@ -29,7 +29,7 @@
     "postbuild": "npx rollup -c ./config/rollup.config.js",
     "watch": "npx tsc-watch --onSuccess \"npm run postbuild\" -p ./config",
     "predeploy": "npm run build",
-    "deploy": "npm publish",
+    "deploy": "npm publish --tag beta",
     "test": "npx jest --config ../../config/jest.config.js --testPathPattern packages/components",
     "test:watch": "npx jest --config ../../config/jest.config.js --testPathPattern packages/components --watch",
     "test:cjs": "npm run build && npx jest --config ../../config/jest.cjs.config.js --testPathPattern packages/components",

diff --git a/packages/hoc/package.json b/packages/hoc/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@apollo/react-hoc",
   "description": "React Apollo `graphql` higher-order component.",
-  "version": "3.1.1",
+  "version": "3.1.2-beta.0",
   "author": "opensource@apollographql.com",
   "keywords": [
     "apollo",
@@ -29,7 +29,7 @@
     "postbuild": "npx rollup -c ./config/rollup.config.js",
     "watch": "npx tsc-watch --onSuccess \"npm run postbuild\" -p ./config",
     "predeploy": "npm run build",
-    "deploy": "npm publish",
+    "deploy": "npm publish --tag beta",
     "test": "npx jest --config ../../config/jest.config.js --testPathPattern packages/hoc",
     "test:watch": "npx jest --config ../../config/jest.config.js --testPathPattern packages/hoc --watch",
     "test:cjs": "npm run build && npx jest --config ../../config/jest.cjs.config.js --testPathPattern packages/hoc",

diff --git a/packages/hooks/package.json b/packages/hooks/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@apollo/react-hooks",
   "description": "React Apollo Hooks.",
-  "version": "3.1.1",
+  "version": "3.1.2-beta.0",
   "author": "opensource@apollographql.com",
   "keywords": [
     "apollo",
@@ -29,7 +29,7 @@
     "postbuild": "npx rollup -c ./config/rollup.config.js",
     "watch": "npx tsc-watch --onSuccess \"npm run postbuild\" -p ./config",
     "predeploy": "npm run build",
-    "deploy": "npm publish",
+    "deploy": "npm publish --tag beta",
     "test": "npx jest --config ../../config/jest.config.js --testPathPattern packages/hooks",
     "test:watch": "npx jest --config ../../config/jest.config.js --testPathPattern packages/hooks --watch",
     "test:cjs": "npm run build && npx jest --config ../../config/jest.cjs.config.js --testPathPattern packages/hooks",

diff --git a/packages/hooks/src/__tests__/useLazyQuery.test.tsx b/packages/hooks/src/__tests__/useLazyQuery.test.tsx
@@ -415,7 +415,7 @@ describe('useLazyQuery Hook', () => {
           expect(loading).toEqual(false);
           expect(data).toEqual(CAR_RESULT_DATA);
           setTimeout(() => {
-            execute();
+            execute({ variables: { someProp: 'someValue' } });
           });
           break;
         case 3:

diff --git a/packages/hooks/src/data/OperationData.ts b/packages/hooks/src/data/OperationData.ts
@@ -30,8 +30,11 @@ export abstract class OperationData<TOptions = any> {
     return this.options;
   }
 
-  public setOptions(newOptions: CommonOptions<TOptions>) {
-    if (!isEqual(this.options, newOptions)) {
+  public setOptions(
+    newOptions: CommonOptions<TOptions>,
+    storePrevious: boolean = false
+  ) {
+    if (storePrevious && !isEqual(this.options, newOptions)) {
       this.previousOptions = this.options;
     }
     this.options = newOptions;

diff --git a/packages/hooks/src/data/QueryData.ts b/packages/hooks/src/data/QueryData.ts
@@ -48,7 +48,7 @@ export class QueryData<TData, TVariables> extends OperationData {
   public execute(): QueryResult<TData, TVariables> {
     this.refreshClient();
 
-    const { skip, query, ssr } = this.getOptions();
+    const { skip, query } = this.getOptions();
     if (skip || query !== this.previousData.query) {
       this.removeQuerySubscription();
       this.previousData.query = query;
@@ -58,9 +58,7 @@ export class QueryData<TData, TVariables> extends OperationData {
 
     if (this.isMounted) this.startQuerySubscription();
 
-    const ssrDisabled = ssr === false;
-
-    return this.getExecuteSsrResult(ssrDisabled) || this.getExecuteResult();
+    return this.getExecuteSsrResult() || this.getExecuteResult();
   }
 
   public executeLazy(): QueryTuple<TData, TVariables> {
@@ -90,6 +88,7 @@ export class QueryData<TData, TVariables> extends OperationData {
 
   public afterExecute({ lazy = false }: { lazy?: boolean } = {}) {
     this.isMounted = true;
+
     if (!lazy || this.runLazy) {
       this.handleErrorOrCompleted();
 
@@ -103,6 +102,7 @@ export class QueryData<TData, TVariables> extends OperationData {
       });
     }
 
+    this.previousOptions = this.getOptions();
     return this.unmount.bind(this);
   }
 
@@ -114,25 +114,24 @@ export class QueryData<TData, TVariables> extends OperationData {
 
   public getOptions() {
     const options = super.getOptions();
-    const lazyOptions = this.lazyOptions || {};
-    const updatedOptions = {
-      ...options,
-      variables: {
+
+    if (this.lazyOptions) {
+      options.variables = {
         ...options.variables,
-        ...lazyOptions.variables
-      },
-      context: {
+        ...this.lazyOptions.variables
+      };
+      options.context = {
         ...options.context,
-        ...lazyOptions.context
-      }
-    };
+        ...this.lazyOptions.context
+      };
+    }
 
     // skip is not supported when using lazy query execution.
     if (this.runLazy) {
-      delete updatedOptions.skip;
+      delete options.skip;
     }
 
-    return updatedOptions;
+    return options;
   }
 
   private runLazyQuery = (options?: QueryLazyOptions<TVariables>) => {
@@ -149,29 +148,31 @@ export class QueryData<TData, TVariables> extends OperationData {
     return result;
   };
 
-  private getExecuteSsrResult(ssrDisabled: boolean) {
-    let result;
-
-    if (this.context && this.context.renderPromises) {
-      const ssrLoading = {
-        loading: true,
-        networkStatus: NetworkStatus.loading,
-        called: true,
-        data: undefined
-      };
-
-      // SSR is disabled, so just return the loading event and leave it in that state.
-      if (ssrDisabled) {
-        return ssrLoading;
-      }
+  private getExecuteSsrResult() {
+    const treeRenderingInitiated = this.context && this.context.renderPromises;
+    const ssrDisabled = this.getOptions().ssr === false;
+    const fetchDisabled = this.refreshClient().client.disableNetworkFetches;
+
+    const ssrLoading = {
+      loading: true,
+      networkStatus: NetworkStatus.loading,
+      called: true,
+      data: undefined
+    } as QueryResult<TData, TVariables>;
+
+    // If SSR has been explicitly disabled, and this function has been called
+    // on the server side, return the default loading state.
+    if (ssrDisabled && (treeRenderingInitiated || fetchDisabled)) {
+      return ssrLoading;
+    }
 
-      result = this.context.renderPromises.addQueryPromise(
-        this,
-        this.getExecuteResult
-      );
-      if (!result) {
-        result = ssrLoading as QueryResult<TData, TVariables>;
-      }
+    let result;
+    if (treeRenderingInitiated) {
+      result =
+        this.context.renderPromises!.addQueryPromise(
+          this,
+          this.getExecuteResult
+        ) || ssrLoading;
     }
 
     return result;
@@ -196,7 +197,7 @@ export class QueryData<TData, TVariables> extends OperationData {
     return {
       ...options,
       displayName,
-      context: options.context || {},
+      context: options.context,
       metadata: { reactComponent: { displayName } }
     };
   }
@@ -213,13 +214,14 @@ export class QueryData<TData, TVariables> extends OperationData {
 
     if (!this.currentObservable.query) {
       const observableQueryOptions = this.prepareObservableQueryOptions();
+
       this.previousData.observableQueryOptions = {
         ...observableQueryOptions,
         children: null
       };
-      this.currentObservable.query = this.refreshClient().client.watchQuery(
-        observableQueryOptions
-      );
+      this.currentObservable.query = this.refreshClient().client.watchQuery({
+        ...observableQueryOptions
+      });
 
       if (this.context && this.context.renderPromises) {
         this.context.renderPromises.registerSSRObservable(
@@ -266,7 +268,6 @@ export class QueryData<TData, TVariables> extends OperationData {
     this.currentObservable.subscription = obsQuery.subscribe({
       next: ({ loading, networkStatus, data }) => {
         const previousResult = this.previousData.result;
-
         if (previousResult) {
           // Calls to `ObservableQuery.fetchMore` return a result before the
           // `updateQuery` function fully finishes. This can lead to an
@@ -282,7 +283,6 @@ export class QueryData<TData, TVariables> extends OperationData {
           ) {
             return;
           }
-
           // Make sure we're not attempting to re-render similar results
           if (
             previousResult.loading === loading &&

diff --git a/packages/hooks/src/useSubscription.ts b/packages/hooks/src/useSubscription.ts
@@ -32,7 +32,7 @@ export function useSubscription<TData = any, TVariables = OperationVariables>(
   }
 
   const subscriptionData = getSubscriptionDataRef();
-  subscriptionData.setOptions(updatedOptions);
+  subscriptionData.setOptions(updatedOptions, true);
   subscriptionData.context = context;
 
   useEffect(() => subscriptionData.afterExecute());

diff --git a/packages/ssr/package.json b/packages/ssr/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@apollo/react-ssr",
   "description": "React Apollo server-side rendering utilities",
-  "version": "3.1.1",
+  "version": "3.1.2-beta.0",
   "author": "opensource@apollographql.com",
   "keywords": [
     "apollo",
@@ -29,7 +29,7 @@
     "postbuild": "npx rollup -c ./config/rollup.config.js",
     "watch": "npx tsc-watch --onSuccess \"npm run postbuild\" -p ./config",
     "predeploy": "npm run build",
-    "deploy": "npm publish",
+    "deploy": "npm publish --tag beta",
     "test": "npx jest --config ../../config/jest.config.js --testPathPattern packages/ssr",
     "test:watch": "npx jest --config ../../config/jest.config.js --testPathPattern packages/ssr --watch",
     "test:cjs": "npm run build && npx jest --config ../../config/jest.cjs.config.js --testPathPattern packages/ssr",