Fix useSubscription executes skipped subscription if input changes

apollographql · Jan 21, 2022 · c4a0a40 · c4a0a40
1 parent a3e8ae0
commit c4a0a40
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 15 deletions.
diff --git a/src/react/hooks/__tests__/useSubscription.test.tsx b/src/react/hooks/__tests__/useSubscription.test.tsx
@@ -127,19 +127,25 @@ describe('useSubscription Hook', () => {
       }
     `;
 
+    const onSetup = jest.fn();
     const link = new MockSubscriptionLink();
+    link.onSetup(onSetup);
     const client = new ApolloClient({
       link,
       cache: new Cache({ addTypename: false })
     });
 
-    const onSubscriptionData = jest.fn();
-    const { result, unmount, waitForNextUpdate } = renderHook(
-      () => useSubscription(subscription, {
-        onSubscriptionData,
-        skip: true,
+    const { result, unmount, waitForNextUpdate, rerender } = renderHook(
+      ({ variables }) => useSubscription(subscription, {
+        variables,
+        skip: true
       }),
       {
+        initialProps: {
+          variables: {
+            foo: 'bar'
+          }
+        },
         wrapper: ({ children }) => (
           <ApolloProvider client={client}>
             {children}
@@ -151,11 +157,13 @@ describe('useSubscription Hook', () => {
     expect(result.current.loading).toBe(false);
     expect(result.current.error).toBe(undefined);
     expect(result.current.data).toBe(undefined);
+
+    rerender({ variables: { foo: 'bar2' }});
+    expect(onSetup).toHaveBeenCalledTimes(0);
+
     await expect(waitForNextUpdate({ timeout: 20 }))
       .rejects.toThrow('Timed out');
     unmount();
-
-    expect(onSubscriptionData).toHaveBeenCalledTimes(0);
   });
 
   it('should create a subscription after skip has changed from true to a falsy value', async () => {

diff --git a/src/react/hooks/useSubscription.ts b/src/react/hooks/useSubscription.ts
@@ -45,14 +45,16 @@ export function useSubscription<TData = any, TVariables = OperationVariables>(
       shouldResubscribe = !!shouldResubscribe(options!);
     }
 
-    if (options?.skip && !options?.skip !== !ref.current.options?.skip) {
-      setResult({
-        loading: false,
-        data: void 0,
-        error: void 0,
-        variables: options?.variables,
-      });
-      setObservable(null);
+    if (options?.skip) {
+      if (!options?.skip !== !ref.current.options?.skip) {
+        setResult({
+          loading: false,
+          data: void 0,
+          error: void 0,
+          variables: options?.variables,
+        });
+        setObservable(null);
+      }
     } else if (
       shouldResubscribe !== false && (
         client !== ref.current.client ||