react-graphql improvements

packages/react-graphql/src/hooks/graphql-document.ts

@@ -2,6 +2,7 @@ import {useState, useEffect, useCallback} from 'react';
 import {OperationVariables} from 'apollo-client';
 import {DocumentNode} from 'graphql-typed';
 import {useMountedRef} from '@shopify/react-hooks';
+import {useAsyncAsset} from '@shopify/react-async';
 
 import {AsyncQueryComponentType} from '../types';
 
@@ -13,7 +14,7 @@ export default function useGraphQLDocument<
   documentOrComponent:
     | DocumentNode<Data, Variables>
     | AsyncQueryComponentType<Data, Variables, DeepPartial>,
-): [DocumentNode<Data, Variables> | null, string | undefined] {
+): DocumentNode<Data, Variables> | null {
   const [document, setDocument] = useState<DocumentNode<
     Data,
     Variables
@@ -52,10 +53,11 @@ export default function useGraphQLDocument<
     [document, loadDocument],
   );
 
-  return [
-    document,
+  useAsyncAsset(
     isDocumentNode(documentOrComponent) ? undefined : documentOrComponent.id,
-  ];
+  );
+
+  return document;
 }
 
 function isDocumentNode(arg: any): arg is DocumentNode {

packages/react-graphql/src/hooks/query.ts

@@ -1,12 +1,15 @@
+/* eslint react-hooks/rules-of-hooks: off */
+
 import {useEffect, useMemo, useState, useRef} from 'react';
 import {
+  ApolloClient,
   OperationVariables,
   ApolloError,
   WatchQueryOptions,
+  ObservableQuery,
 } from 'apollo-client';
 import {DocumentNode} from 'graphql-typed';
 import {useServerEffect} from '@shopify/react-effect';
-import {useAsyncAsset} from '@shopify/react-async';
 
 import {AsyncQueryComponentType} from '../types';
 import {QueryHookOptions, QueryHookResult} from './types';
@@ -33,12 +36,15 @@ export default function useQuery<
     notifyOnNetworkStatusChange,
     context,
   } = options;
-
   const client = useApolloClient(overrideClient);
-  const [query, id] = useGraphQLDocument(queryOrComponent);
 
-  useAsyncAsset(id);
+  if (typeof window === 'undefined' && skip) {
+    return createDefaultResult(client, variables);
+  }
 
+  const query = useGraphQLDocument(queryOrComponent);
+
+  const serializedVariables = variables && JSON.stringify(variables);
   const watchQueryOptions = useMemo<WatchQueryOptions<Variables> | null>(
     () => {
       if (!query) {
@@ -60,8 +66,7 @@ export default function useQuery<
       query,
       // eslint-disable-next-line react-hooks/exhaustive-deps
       context && JSON.stringify(context),
-      // eslint-disable-next-line react-hooks/exhaustive-deps
-      variables && JSON.stringify(variables),
+      serializedVariables,
       fetchPolicy,
       errorPolicy,
       pollInterval,
@@ -90,33 +95,9 @@ export default function useQuery<
   });
 
   const defaultResult = useMemo<QueryHookResult<Data, Variables>>(
-    () => ({
-      data: undefined,
-      error: undefined,
-      networkStatus: undefined,
-      loading: false,
-      variables: queryObservable ? queryObservable.variables : variables,
-      refetch: queryObservable
-        ? queryObservable.refetch.bind(queryObservable)
-        : (noop as any),
-      fetchMore: queryObservable
-        ? queryObservable.fetchMore.bind(queryObservable)
-        : (noop as any),
-      updateQuery: queryObservable
-        ? queryObservable.updateQuery.bind(queryObservable)
-        : (noop as any),
-      startPolling: queryObservable
-        ? queryObservable.startPolling.bind(queryObservable)
-        : (noop as any),
-      stopPolling: queryObservable
-        ? queryObservable.stopPolling.bind(queryObservable)
-        : (noop as any),
-      subscribeToMore: queryObservable
-        ? queryObservable.subscribeToMore.bind(queryObservable)
-        : (noop as any),
-      client,
-    }),
-    [queryObservable, client, variables],
+    () => createDefaultResult(client, variables, queryObservable),
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [queryObservable, client, serializedVariables],
   );
 
   const [responseId, setResponseId] = useState(0);
@@ -145,6 +126,7 @@ export default function useQuery<
   const previousData = useRef<
     QueryHookResult<Data, Variables>['data'] | undefined
   >(undefined);
+
   const currentResult = useMemo<QueryHookResult<Data, Variables>>(
     () => {
       // must of the logic below are lifted from
@@ -202,4 +184,37 @@ export default function useQuery<
   return currentResult;
 }
 
+function createDefaultResult(
+  client: ApolloClient<unknown>,
+  variables: any,
+  queryObservable?: ObservableQuery,
+) {
+  return {
+    data: undefined,
+    error: undefined,
+    networkStatus: undefined,
+    loading: false,
+    variables: queryObservable ? queryObservable.variables : variables,
+    refetch: queryObservable
+      ? queryObservable.refetch.bind(queryObservable)
+      : noop,
+    fetchMore: queryObservable
+      ? queryObservable.fetchMore.bind(queryObservable)
+      : noop,
+    updateQuery: queryObservable
+      ? queryObservable.updateQuery.bind(queryObservable)
+      : noop,
+    startPolling: queryObservable
+      ? queryObservable.startPolling.bind(queryObservable)
+      : noop,
+    stopPolling: queryObservable
+      ? queryObservable.stopPolling.bind(queryObservable)
+      : noop,
+    subscribeToMore: queryObservable
+      ? queryObservable.subscribeToMore.bind(queryObservable)
+      : noop,
+    client,
+  };
+}
+
 function noop() {}

packages/react-graphql/src/hooks/tests/query.test.tsx

@@ -7,12 +7,7 @@ import {createGraphQLFactory} from '@shopify/graphql-testing';
 
 import {createAsyncQueryComponent} from '../../async';
 import useQuery from '../query';
-import {
-  mountWithGraphQL,
-  prepareAsyncReactTasks,
-  teardownAsyncReactTasks,
-  createResolvablePromise,
-} from './utilities';
+import {mountWithGraphQL, createResolvablePromise} from './utilities';
 
 const petQuery = gql`
   query PetQuery {
@@ -33,14 +28,6 @@ const mockData = {
 };
 
 describe('useQuery', () => {
-  beforeEach(() => {
-    prepareAsyncReactTasks();
-  });
-
-  afterEach(() => {
-    teardownAsyncReactTasks();
-  });
-
   describe('document', () => {
     it('returns loading=true and networkStatus=loading during the loading of query', async () => {
       function MockQuery({children}) {
@@ -89,6 +76,43 @@ describe('useQuery', () => {
       );
     });
 
+    it('keeps the same data when the variables stay deep-equal', async () => {
+      function MockQuery({
+        children,
+        variables,
+      }: {
+        children: (
+          result: ReturnType<typeof useQuery>,
+        ) => React.ReactElement | null;
+        variables?: object;
+      }) {
+        const results = useQuery(petQuery, {variables});
+        return children(results);
+      }
+
+      const graphQL = createGraphQL({PetQuery: mockData});
+      const renderPropSpy = jest.fn(() => null);
+      const variables = {foo: 'bar'};
+
+      const mockQuery = await mountWithGraphQL(
+        <MockQuery variables={variables}>{renderPropSpy}</MockQuery>,
+        {
+          graphQL,
+        },
+      );
+
+      mockQuery.setProps({variables: {...variables}});
+
+      expect(graphQL.operations.all()).toHaveLength(1);
+
+      // Once for initial render while loading, once for when the data loaded, and a final time
+      // when we update the props and re-render the component.
+      expect(renderPropSpy).toHaveBeenCalledTimes(3);
+
+      const [, firstLoadedCall, secondLoadedCall] = renderPropSpy.mock.calls;
+      expect(firstLoadedCall[0]).toBe(secondLoadedCall[0]);
+    });
+
     it('watchQuery is not called when skip is true', async () => {
       const mockClient = createMockApolloClient();
       const watchQuerySpy = jest.fn();

packages/react-graphql/src/hooks/tests/utilities.tsx

@@ -2,7 +2,6 @@ import * as React from 'react';
 
 import {createGraphQLFactory, GraphQL} from '@shopify/graphql-testing';
 import {createMount} from '@shopify/react-testing';
-import {promise} from '@shopify/jest-dom-mocks';
 
 import {ApolloProvider} from '../../ApolloProvider';
 
@@ -37,28 +36,6 @@ export const mountWithGraphQL = createMount<Options, Context, true>({
   },
 });
 
-export function prepareAsyncReactTasks() {
-  if (!promise.isMocked()) {
-    promise.mock();
-  }
-}
-
-export function teardownAsyncReactTasks() {
-  if (promise.isMocked()) {
-    promise.restore();
-  }
-}
-
-export function runPendingAsyncReactTasks() {
-  if (!promise.isMocked()) {
-    throw new Error(
-      'You attempted to resolve pending async React tasks, but have not yet prepared to do so. Run `prepareAsyncReactTasks()` from "tests/modern" in a `beforeEach` block and try again.',
-    );
-  }
-
-  promise.runPending();
-}
-
 export function createResolvablePromise<T>(value: T) {
   let resolver!: () => Promise<T>;
   let rejecter!: () => void;

packages/react-testing/src/TestWrapper.tsx

@@ -6,6 +6,7 @@ interface State<ChildProps> {
 
 interface Props {
   children: React.ReactElement<any>;
+  render(element: React.ReactElement<any>): React.ReactElement<any>;
 }
 
 export class TestWrapper<ChildProps> extends React.Component<
@@ -21,7 +22,7 @@ export class TestWrapper<ChildProps> extends React.Component<
 
   render() {
     const {props} = this.state;
-    const {children} = this.props;
-    return props ? React.cloneElement(children, props) : children;
+    const {children, render} = this.props;
+    return render(props ? React.cloneElement(children, props) : children);
   }
 }

packages/react-testing/src/mount.ts

@@ -1,6 +1,6 @@
 import * as React from 'react';
 import {IfAllOptionalKeys} from '@shopify/useful-types';
-import {Root} from './root';
+import {Root, Options as RootOptions} from './root';
 import {Element} from './element';
 
 export {Root, Element};
@@ -81,9 +81,9 @@ export class CustomRoot<Props, Context extends object> extends Root<Props> {
   constructor(
     tree: React.ReactElement<Props>,
     public readonly context: Context,
-    resolve: (element: Element<unknown>) => Element<unknown> | null,
+    options?: RootOptions,
   ) {
-    super(tree, resolve);
+    super(tree, options);
   }
 }
 
@@ -105,11 +105,11 @@ export function createMount<
     options: MountOptions = {} as any,
   ) {
     const context = createContext(options);
-    const rendered = render(element, context, options);
 
-    const wrapper = new CustomRoot(rendered, context, root =>
-      root.find(element.type),
-    );
+    const wrapper = new CustomRoot(element, context, {
+      render: element => render(element, context, options),
+      resolveRoot: root => root.find(element.type),
+    });
 
     const afterMountResult = afterMount(wrapper, options);
 

packages/react-testing/src/root.tsx

@@ -26,6 +26,14 @@ const act = reactAct as (func: () => void | Promise<void>) => Promise<void>;
 const {findCurrentFiberUsingSlowPath} = require('react-reconciler/reflection');
 
 type ResolveRoot = (element: Element<unknown>) => Element<unknown> | null;
+type Render = (
+  element: React.ReactElement<unknown>,
+) => React.ReactElement<unknown>;
+
+export interface Options {
+  render?: Render;
+  resolveRoot?: ResolveRoot;
+}
 
 export const connected = new Set<Root<unknown>>();
 
@@ -67,14 +75,20 @@ export class Root<Props> {
   private root: Element<Props> | null = null;
   private acting = false;
 
+  private render: Render;
+  private resolveRoot: ResolveRoot;
+
   private get mounted() {
     return this.wrapper != null;
   }
 
   constructor(
     private tree: React.ReactElement<Props>,
-    private resolveRoot: ResolveRoot = defaultResolveRoot,
+    {render = defaultRender, resolveRoot = defaultResolveRoot}: Options = {},
   ) {
+    this.render = render;
+    this.resolveRoot = resolveRoot;
+
     this.mount();
   }
 
@@ -179,6 +193,7 @@ export class Root<Props> {
     this.act(() => {
       render(
         <TestWrapper<Props>
+          render={this.render}
           ref={wrapper => {
             this.wrapper = wrapper;
           }}
@@ -257,6 +272,10 @@ function defaultResolveRoot(element: Element<unknown>) {
   return element.children[0];
 }
 
+function defaultRender(element: React.ReactElement<unknown>) {
+  return element;
+}
+
 function flatten(
   element: Fiber,
   root: Root<unknown>,