feat(@apollo/react-hooks): implement skip option for useSubscription hook

[n1ru4l]

This is a possible implementation for apollographql/apollo-feature-requests#121

Would love some recommendations on how to write a test that verifies a subscription is canceled after the value of skip has changed.

Checklist:

• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.

patch-package patch:

patches/@apollo+react-hooks+3.0.1.patch

diff --git a/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.d.ts b/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.d.ts
index 377988f..798e561 100644
--- a/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.d.ts
+++ b/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.d.ts
@@ -20,7 +20,8 @@ export declare class SubscriptionData<TData = any, TVariables = any> extends Ope
     private initialize;
     private startSubscription;
     private getLoadingResult;
-    private updateResult;
+    private getLoadingResult;
+    private getSkipResult;
     private updateCurrentData;
     private updateError;
     private completeSubscription;
diff --git a/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.js b/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.js
index b342124..3e43a79 100644
--- a/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.js
+++ b/node_modules/@apollo/react-hooks/lib/data/SubscriptionData.js
@@ -13,6 +13,12 @@ var SubscriptionData = (function (_super) {
     }
     SubscriptionData.prototype.execute = function (result) {
         var currentResult = result;
+        if (this.getOptions().skip === true) {
+            this.endSubscription();
+            delete this.currentObservable.query;
+            currentResult = this.getSkipResult();
+            return { ...currentResult, variables: this.getOptions().variables };
+        }
         if (this.refreshClient().isNew) {
             currentResult = this.getLoadingResult();
         }
@@ -29,6 +35,15 @@ var SubscriptionData = (function (_super) {
             delete this.currentObservable.query;
             currentResult = this.getLoadingResult();
         }
+
+        if (
+            this.previousOptions.skip === true &&
+            !this.getOptions().skip
+        ) {
+            currentResult.loading = true;
+            currentResult.data = undefined;
+        }
+
         this.initialize(this.getOptions());
         this.startSubscription();
         this.previousOptions = this.getOptions();
@@ -42,8 +57,7 @@ var SubscriptionData = (function (_super) {
         delete this.currentObservable.query;
     };
     SubscriptionData.prototype.initialize = function (options) {
-        if (this.currentObservable.query)
-            return;
+        if (this.currentObservable.query || this.getOptions().skip === true) return;
         this.currentObservable.query = this.refreshClient().client.subscribe({
             query: options.subscription,
             variables: options.variables,
@@ -66,6 +80,13 @@ var SubscriptionData = (function (_super) {
             data: undefined
         };
     };
+    SubscriptionData.prototype.getSkipResult = function (result) {
+        return {
+            loading: false,
+            error: undefined,
+            data: undefined
+        };
+    };
     SubscriptionData.prototype.updateResult = function (result) {
         if (this.isMounted) {
             this.setResult(result);
diff --git a/node_modules/@apollo/react-hooks/lib/useSubscription.js b/node_modules/@apollo/react-hooks/lib/useSubscription.js
index a1f92a7..63ce176 100644
--- a/node_modules/@apollo/react-hooks/lib/useSubscription.js
+++ b/node_modules/@apollo/react-hooks/lib/useSubscription.js
@@ -4,13 +4,13 @@ import { getApolloContext } from '@apollo/react-common';
 import { SubscriptionData } from './data/SubscriptionData';
 export function useSubscription(subscription, options) {
     var context = useContext(getApolloContext());
+    var updatedOptions = options
+        ? tslib_1.__assign({}, options, { subscription: subscription }) : { subscription: subscription };
     var _a = useState({
-        loading: true,
+        loading: !updatedOptions.skip,
         error: undefined,
         data: undefined
     }), result = _a[0], setResult = _a[1];
-    var updatedOptions = options
-        ? tslib_1.__assign({}, options, { subscription: subscription }) : { subscription: subscription };
     var subscriptionDataRef = useRef();
     function getSubscriptionDataRef() {
         if (!subscriptionDataRef.current) {

patches/@apollo+react-common+3.0.1.patch

diff --git a/node_modules/@apollo/react-common/lib/types/types.d.ts b/node_modules/@apollo/react-common/lib/types/types.d.ts
index cfee4c8..9d6b9b9 100644
--- a/node_modules/@apollo/react-common/lib/types/types.d.ts
+++ b/node_modules/@apollo/react-common/lib/types/types.d.ts
@@ -84,6 +84,7 @@ export interface BaseSubscriptionOptions<TData = any, TVariables = OperationVari
     fetchPolicy?: FetchPolicy;
     shouldResubscribe?: boolean | ((options: BaseSubscriptionOptions<TData, TVariables>) => boolean);
     client?: ApolloClient<object>;
+    skip?: boolean;
     onSubscriptionData?: (options: OnSubscriptionDataOptions<TData>) => any;
     onSubscriptionComplete?: () => void;
 }

[n1ru4l]

Hey @hwillson, the absence of the skip option is currently blocking me (and others) from migrating over from react-apollo-hooks.

Of course, I can understand that you might be busy with different stuff, but I would be grateful if you got time to review and point this pull request in the right direction. 😊

[hwillson]

Thanks for working on this @n1ru4l - I'll definitely get this reviewed today.

[hwillson]

Thanks very much for working on this @n1ru4l 🙇. I think we're good to go here, so LGTM!

[matthewhuang-camelot]

Thanks for this @n1ru4l, looking forward to having it released! As for my use case, I need this for SSR with Next.js -- currently there's no way via the useSubscription hook to tell SSR-code to not try and initiate a subscription (subscriptions should only be initialized on the browser after mount).

So I'm going to be wrapping Apollo's useSubscription in a custom hook that has skip: true if it's in SSR mode. Any thoughts on alternatives, or would people recommend the same?

[jurajkrivda]

@matthewhuang-camelot Could you please share your custom hook? Thanks

[matthewhuang97]

@jurajkrivda I was thinking a simple wrapper like this:

function _useSubscription(subscription, options) {
  return useSubscription(subscription, { ...options, skip: IS_SSR });
}

However, since the skip flag hadn't been released at the time I needed it, I went with this instead:

function _useSubscription(...args) {
  if (IS_SSR) return { loading: false, data: undefined, error: undefined };
  // eslint-disable-next-line react-hooks/rules-of-hooks
  else return useSubscription(...args);
}

This works out in Next.js SSR for Apollo -- when the initial render is run, the hook returns a dummy result + never calls the Apollo useSubscription. There's actually no issue here with violating the no-conditional-hooks rule -- in the SSR runtime, useSubscription is never run, and on the browser, useSubscription is always run!

[jedwards1211]

@matthewhuang97 what kind of apollo link are you using on the server side? apollo-link-schema doesn't perform subscriptions anyway. There's a PR for that, but it would be an opt-in thing only for testing, because obviously there's little point in doing subs during SSR.