Repository: apollographql/apollo-client

Problem statement:
change `onSubscriptionData` and `onSubscriptionComplete` to `onData` and `onComplete`
- relates to #9495 
- see [comment](https://github.com/apollographql/apollo-client/pull/9495/files#r866194974)

As  a follow up to #9495 we want to change `onSubscriptionData` and `onSubscriptionComplete` to `onData` and `onComplete` so we are consistent in our types naming convention.





Hints:
@jpvajda I'd like to consider taking this on. Since `onSubscriptionData` and `onSubscriptionComplete` are currently in the stable API, I assume we'd want to deprecate these existing APIs (by showing a deprecation message) in addition to adding the new `onData` and `onComplete`.

Does this sound correct?

Should we allow users to use both together? My initial thinking is no, since one will replace the other. Perhaps we error in this situation?

Patch:
diff --git a/src/react/hooks/useSubscription.ts b/src/react/hooks/useSubscription.ts
--- a/src/react/hooks/useSubscription.ts
+++ b/src/react/hooks/useSubscription.ts
@@ -2,6 +2,7 @@ import '../../utilities/globals';
 import { useState, useRef, useEffect } from 'react';
 import { DocumentNode } from 'graphql';
 import { TypedDocumentNode } from '@graphql-typed-document-node/core';
+import { invariant } from '../../utilities/globals'
 import { equal } from '@wry/equality';
 
 import { DocumentType, verifyDocumentType } from '../parser';
@@ -16,6 +17,7 @@ export function useSubscription<TData = any, TVariables = OperationVariables>(
   subscription: DocumentNode | TypedDocumentNode<TData, TVariables>,
   options?: SubscriptionHookOptions<TData, TVariables>,
 ) {
+  const hasIssuedDeprecationWarningRef = useRef(false);
   const client = useApolloClient(options?.client);
   verifyDocumentType(subscription, DocumentType.Subscription);
   const [result, setResult] = useState<SubscriptionResult<TData>>({
@@ -25,6 +27,26 @@ export function useSubscription<TData = any, TVariables = OperationVariables>(
     variables: options?.variables,
   });
 
+  if (!hasIssuedDeprecationWarningRef.current) {
+    hasIssuedDeprecationWarningRef.current = true;
+
+    if (options?.onSubscriptionData) {
+      invariant.warn(
+        options.onData
+          ? "'useSubscription' supports only the 'onSubscriptionData' or 'onData' option, but not both. Only the 'onData' option will be used."
+          : "'onSubscriptionData' is deprecated and will be removed in a future major version. Please use the 'onData' option instead."
+      );
+    }
+
+    if (options?.onSubscriptionComplete) {
+      invariant.warn(
+        options.onComplete
+          ? "'useSubscription' supports only the 'onSubscriptionComplete' or 'onComplete' option, but not both. Only the 'onComplete' option will be used."
+          : "'onSubscriptionComplete' is deprecated and will be removed in a future major version. Please use the 'onComplete' option instead."
+      );
+    }
+  }
+
   const [observable, setObservable] = useState(() => {
     if (options?.skip) {
       return null;
@@ -107,10 +129,17 @@ export function useSubscription<TData = any, TVariables = OperationVariables>(
         };
         setResult(result);
 
-        ref.current.options?.onSubscriptionData?.({
-          client,
-          subscriptionData: result
-        });
+        if (ref.current.options?.onData) {
+          ref.current.options.onData({
+            client,
+            data: result
+          });
+        } else if (ref.current.options?.onSubscriptionData) {
+          ref.current.options.onSubscriptionData({
+            client,
+            subscriptionData: result
+          });
+        }
       },
       error(error) {
         setResult({
@@ -122,7 +151,11 @@ export function useSubscription<TData = any, TVariables = OperationVariables>(
         ref.current.options?.onError?.(error);
       },
       complete() {
-        ref.current.options?.onSubscriptionComplete?.();
+        if (ref.current.options?.onComplete) {
+          ref.current.options.onComplete();
+        } else if (ref.current.options?.onSubscriptionComplete) {
+          ref.current.options.onSubscriptionComplete();
+        }
       },
     });
 
diff --git a/src/react/types/types.ts b/src/react/types/types.ts
--- a/src/react/types/types.ts
+++ b/src/react/types/types.ts
@@ -202,6 +202,11 @@ export type MutationTuple<
 
 /* Subscription types */
 
+export interface OnDataOptions<TData = any> {
+  client: ApolloClient<object>;
+  data: SubscriptionResult<TData>;
+}
+
 export interface OnSubscriptionDataOptions<TData = any> {
   client: ApolloClient<object>;
   subscriptionData: SubscriptionResult<TData>;
@@ -219,8 +224,16 @@ export interface BaseSubscriptionOptions<
   client?: ApolloClient<object>;
   skip?: boolean;
   context?: DefaultContext;
+  onComplete?: () => void;
+  onData?: (options: OnDataOptions<TData>) => any;
+  /**
+  * @deprecated Use onData instead
+  */
   onSubscriptionData?: (options: OnSubscriptionDataOptions<TData>) => any;
   onError?: (error: ApolloError) => void;
+  /**
+  * @deprecated Use onComplete instead
+  */
   onSubscriptionComplete?: () => void;
 }
 


Test patch:
diff --git a/src/react/components/__tests__/client/Subscription.test.tsx b/src/react/components/__tests__/client/Subscription.test.tsx
--- a/src/react/components/__tests__/client/Subscription.test.tsx
+++ b/src/react/components/__tests__/client/Subscription.test.tsx
@@ -88,7 +88,37 @@ itAsync('executes the subscription', (resolve, reject) => {
   waitFor(() => expect(renderCount).toBe(5)).then(resolve, reject);
 });
 
-itAsync('calls onSubscriptionData if given', (resolve, reject) => {
+it('calls onData if given', async () => {
+  let count = 0;
+
+  const Component = () => (
+    <Subscription
+      subscription={subscription}
+      onData={(opts: any) => {
+        expect(opts.client).toBeInstanceOf(ApolloClient);
+        const { data } = opts.data;
+        expect(data).toEqual(results[count].result.data);
+        count++;
+      }}
+    />
+  );
+
+  render(
+    <ApolloProvider client={client}>
+      <Component />
+    </ApolloProvider>
+  );
+
+  const interval = setInterval(() => {
+    link.simulateResult(results[count]);
+    if (count >= 3) clearInterval(interval);
+  }, 10);
+
+  await waitFor(() => expect(count).toBe(4));
+});
+
+it('calls onSubscriptionData with deprecation warning if given', async () => {
+  const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
   let count = 0;
 
   const Component = () => (
@@ -109,22 +139,60 @@ itAsync('calls onSubscriptionData if given', (resolve, reject) => {
     </ApolloProvider>
   );
 
+  expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+  expect(consoleWarnSpy).toHaveBeenCalledWith(
+    expect.stringContaining("'onSubscriptionData' is deprecated")
+  );
+
   const interval = setInterval(() => {
     link.simulateResult(results[count]);
     if (count >= 3) clearInterval(interval);
   }, 10);
 
-  waitFor(() => expect(count).toBe(4)).then(resolve, reject);
+  await waitFor(() => expect(count).toBe(4))
+
+  consoleWarnSpy.mockRestore();
 });
 
-itAsync('should call onSubscriptionComplete if specified', (resolve, reject) => {
+it('should call onComplete if specified', async () => {
   let count = 0;
 
   let done = false;
   const Component = () => (
     <Subscription
       subscription={subscription}
-      onSubscriptionData={() => {
+      onData={() => {
+        count++;
+      }}
+      onComplete={() => {
+        done = true;
+      }}
+    />
+  );
+
+  render(
+    <ApolloProvider client={client}>
+      <Component />
+    </ApolloProvider>
+  );
+
+  const interval = setInterval(() => {
+    link.simulateResult(results[count], count === 3);
+    if (count >= 3) clearInterval(interval);
+  }, 10);
+
+  await waitFor(() => expect(done).toBeTruthy());
+});
+
+it('should call onSubscriptionComplete with deprecation warning if specified', async () => {
+  const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+  let count = 0;
+
+  let done = false;
+  const Component = () => (
+    <Subscription
+      subscription={subscription}
+      onData={() => {
         count++;
       }}
       onSubscriptionComplete={() => {
@@ -139,12 +207,19 @@ itAsync('should call onSubscriptionComplete if specified', (resolve, reject) =>
     </ApolloProvider>
   );
 
+  expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+  expect(consoleWarnSpy).toHaveBeenCalledWith(
+    expect.stringContaining("'onSubscriptionComplete' is deprecated")
+  );
+
   const interval = setInterval(() => {
     link.simulateResult(results[count], count === 3);
     if (count >= 3) clearInterval(interval);
   }, 10);
 
-  waitFor(() => expect(done).toBeTruthy()).then(resolve, reject);
+  await waitFor(() => expect(done).toBeTruthy());
+
+  consoleWarnSpy.mockRestore();
 });
 
 itAsync('executes subscription for the variables passed in the props', (resolve, reject) => {
diff --git a/src/react/hooks/__tests__/useSubscription.test.tsx b/src/react/hooks/__tests__/useSubscription.test.tsx
--- a/src/react/hooks/__tests__/useSubscription.test.tsx
+++ b/src/react/hooks/__tests__/useSubscription.test.tsx
@@ -9,6 +9,10 @@ import { MockSubscriptionLink } from '../../../testing';
 import { useSubscription } from '../useSubscription';
 
 describe('useSubscription Hook', () => {
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
   it('should handle a simple subscription properly', async () => {
     const subscription = gql`
       subscription {
@@ -112,6 +116,45 @@ describe('useSubscription Hook', () => {
     });
   });
 
+  it('should call onComplete after subscription is complete', async () => {
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const results = [{
+      result: { data: { car: { make: 'Audi' } } }
+    }];
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false })
+    });
+
+    const onComplete = jest.fn();
+    const { waitForNextUpdate } = renderHook(
+      () => useSubscription(subscription, { onComplete }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    link.simulateResult(results[0]);
+
+    setTimeout(() => link.simulateComplete());
+    await waitForNextUpdate();
+
+    expect(onComplete).toHaveBeenCalledTimes(1);
+  });
+
   it('should cleanup after the subscription component has been unmounted', async () => {
     const subscription = gql`
       subscription {
@@ -133,10 +176,10 @@ describe('useSubscription Hook', () => {
       cache: new Cache({ addTypename: false })
     });
 
-    const onSubscriptionData = jest.fn();
+    const onData = jest.fn();
     const { result, unmount, waitForNextUpdate } = renderHook(
       () => useSubscription(subscription, {
-        onSubscriptionData,
+        onData,
       }),
       {
         wrapper: ({ children }) => (
@@ -156,17 +199,17 @@ describe('useSubscription Hook', () => {
     expect(result.current.error).toBe(undefined);
     expect(result.current.data).toBe(results[0].result.data);
     setTimeout(() => {
-      expect(onSubscriptionData).toHaveBeenCalledTimes(1);
+      expect(onData).toHaveBeenCalledTimes(1);
       // After the component has been unmounted, the internal
       // ObservableQuery should be stopped, meaning it shouldn't
-      // receive any new data (so the onSubscriptionDataCount should
+      // receive any new data (so the onDataCount should
       // stay at 1).
       unmount();
       link.simulateResult(results[0]);
     });
 
     await new Promise((resolve) => setTimeout(resolve, 100));
-    expect(onSubscriptionData).toHaveBeenCalledTimes(1);
+    expect(onData).toHaveBeenCalledTimes(1);
   });
 
   it('should never execute a subscription with the skip option', async () => {
@@ -186,7 +229,7 @@ describe('useSubscription Hook', () => {
       cache: new Cache({ addTypename: false })
     });
 
-    const onSubscriptionData = jest.fn();
+    const onData = jest.fn();
     const wrapper: React.FC<PropsWithChildren<{ variables: { foo: string } }>> = ({ children }) => (
       <ApolloProvider client={client}>
         {children}
@@ -197,7 +240,7 @@ describe('useSubscription Hook', () => {
       ({ variables }) => useSubscription(subscription, {
         variables,
         skip: true,
-        onSubscriptionData,
+        onData,
       }),
       {
         initialProps: {
@@ -218,7 +261,7 @@ describe('useSubscription Hook', () => {
       .rejects.toThrow('Timed out');
 
     expect(onSetup).toHaveBeenCalledTimes(0);
-    expect(onSubscriptionData).toHaveBeenCalledTimes(0);
+    expect(onData).toHaveBeenCalledTimes(0);
     unmount();
   });
 
@@ -557,4 +600,322 @@ describe('useSubscription Hook', () => {
     );
     errorSpy.mockRestore();
   });
+
+  test("should warn when using 'onSubscriptionData' and 'onData' together", () => {
+    const warningSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    renderHook(
+      () => useSubscription(subscription, {
+        onData: jest.fn(),
+        onSubscriptionData: jest.fn(),
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    expect(warningSpy).toHaveBeenCalledTimes(1);
+    expect(warningSpy).toHaveBeenCalledWith(expect.stringContaining("supports only the 'onSubscriptionData' or 'onData' option"));
+  });
+
+  test("prefers 'onData' when using 'onSubscriptionData' and 'onData' together", async () => {
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const results = [
+      {
+        result: { data: { car: { make: 'Pagani' } } }
+      }
+    ];
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    const onData = jest.fn();
+    const onSubscriptionData = jest.fn();
+
+    const { waitForNextUpdate } = renderHook(
+      () => useSubscription(subscription, {
+        onData,
+        onSubscriptionData,
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    setTimeout(() => link.simulateResult(results[0]));
+    await waitForNextUpdate();
+
+    setTimeout(() => {
+      expect(onData).toHaveBeenCalledTimes(1);
+      expect(onSubscriptionData).toHaveBeenCalledTimes(0);
+    });
+  });
+
+  test("uses 'onSubscriptionData' when 'onData' is absent", async () => {
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const results = [
+      {
+        result: { data: { car: { make: 'Pagani' } } }
+      }
+    ];
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    const onSubscriptionData = jest.fn();
+
+    const { waitForNextUpdate } = renderHook(
+      () => useSubscription(subscription, {
+        onSubscriptionData,
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    setTimeout(() => link.simulateResult(results[0]));
+    await waitForNextUpdate();
+
+    setTimeout(() => {
+      expect(onSubscriptionData).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  test("only warns once using `onSubscriptionData`", () => {
+    const warningSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    const { rerender } = renderHook(
+      () => useSubscription(subscription, {
+        onSubscriptionData: jest.fn(),
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    rerender();
+
+    expect(warningSpy).toHaveBeenCalledTimes(1);
+  });
+
+  test("should warn when using 'onComplete' and 'onSubscriptionComplete' together", () => {
+    const warningSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    renderHook(
+      () => useSubscription(subscription, {
+        onComplete: jest.fn(),
+        onSubscriptionComplete: jest.fn(),
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    expect(warningSpy).toHaveBeenCalledTimes(1);
+    expect(warningSpy).toHaveBeenCalledWith(expect.stringContaining("supports only the 'onSubscriptionComplete' or 'onComplete' option"));
+  });
+
+  test("prefers 'onComplete' when using 'onComplete' and 'onSubscriptionComplete' together", async () => {
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const results = [{
+      result: { data: { car: { make: 'Audi' } } }
+    }];
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    const onComplete = jest.fn();
+    const onSubscriptionComplete = jest.fn();
+
+    const { waitForNextUpdate } = renderHook(
+      () => useSubscription(subscription, {
+        onComplete,
+        onSubscriptionComplete,
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    link.simulateResult(results[0]);
+
+    setTimeout(() => link.simulateComplete());
+    await waitForNextUpdate();
+
+    expect(onComplete).toHaveBeenCalledTimes(1);
+    expect(onSubscriptionComplete).toHaveBeenCalledTimes(0);
+  });
+
+  test("uses 'onSubscriptionComplete' when 'onComplete' is absent", async () => {
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const results = [{
+      result: { data: { car: { make: 'Audi' } } }
+    }];
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    const onSubscriptionComplete = jest.fn();
+
+    const { waitForNextUpdate } = renderHook(
+      () => useSubscription(subscription, {
+        onSubscriptionComplete,
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    link.simulateResult(results[0]);
+
+    setTimeout(() => link.simulateComplete());
+    await waitForNextUpdate();
+
+    expect(onSubscriptionComplete).toHaveBeenCalledTimes(1);
+  });
+
+  test("only warns once using `onSubscriptionComplete`", () => {
+    const warningSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const subscription = gql`
+      subscription {
+        car {
+          make
+        }
+      }
+    `;
+
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new Cache({ addTypename: false }),
+    });
+
+    const { rerender } = renderHook(
+      () => useSubscription(subscription, {
+        onSubscriptionComplete: jest.fn(),
+      }),
+      {
+        wrapper: ({ children }) => (
+          <ApolloProvider client={client}>
+            {children}
+          </ApolloProvider>
+        ),
+      },
+    );
+
+    rerender();
+
+    expect(warningSpy).toHaveBeenCalledTimes(1);
+  });
 });