Repository: FormidableLabs/urql (ts)

Issue:
Typenames are being unnecessarily added
Typenames are added to selections without checking if a typename node is already present.

Example query:
```
{
  todos {
    id
    text
    complete
    __typename
    __typename
    __typename
  }
  __typename
  __typename
  __typename
}
```

Pull request:
add typename existence check
Fix #320 

**Note: ** The [check for a selections array](https://github.com/FormidableLabs/urql/compare/320-fix-typename-additions?expand=1#diff-f56409a2c09e8916f77be7385bb12343L39) inside of the selectionSet has been removed. According to _@types/graphql_ the [selections property is required](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/graphql/language/ast.d.ts#L226).

Patch:
diff --git a/src/hooks/useDevtoolsContext.ts b/src/hooks/useDevtoolsContext.ts
new file mode 100644
index 00000000..c7b57c2c
--- /dev/null
+++ b/src/hooks/useDevtoolsContext.ts
@@ -0,0 +1,28 @@
+import { useMemo, useRef } from 'react';
+
+/** Find the name of the calling component. */
+const getHookParent = () => {
+  /**
+   * Line values could be:
+   * - named export             " at exports.Home (Home.tsx?9b1b:17)"
+   * - default export / inline  " at Home (Home.tsx?9b1b:17)"
+   */
+  const stackLine = (new Error().stack as string).split('\n')[4];
+  const parsedLine = /.*at (exports\.)?(\w+)/.exec(stackLine);
+
+  return parsedLine === null || typeof parsedLine[2] !== 'string'
+    ? 'Unknown'
+    : parsedLine[2];
+};
+
+const useDevtoolsContextHook = () => {
+  const source = useRef(getHookParent());
+
+  return useMemo(() => [{ devtools: { source: source.current } }], []);
+};
+
+/** Creates additional context values for serving metadata to devtools. */
+export const useDevtoolsContext =
+  process.env.NODE_ENV === 'production'
+    ? () => [undefined]
+    : useDevtoolsContextHook;
diff --git a/src/hooks/useMutation.ts b/src/hooks/useMutation.ts
index 3836b6e9..78fcd816 100644
--- a/src/hooks/useMutation.ts
+++ b/src/hooks/useMutation.ts
@@ -5,6 +5,7 @@ import { Context } from '../context';
 import { OperationResult } from '../types';
 import { CombinedError, createRequest } from '../utils';
 import { useImmediateState } from './useImmediateState';
+import { useDevtoolsContext } from './useDevtoolsContext';
 
 export interface UseMutationState<T> {
   fetching: boolean;
@@ -20,6 +21,7 @@ export type UseMutationResponse<T, V> = [
 export const useMutation = <T = any, V = object>(
   query: DocumentNode | string
 ): UseMutationResponse<T, V> => {
+  const [devtoolsContext] = useDevtoolsContext();
   const client = useContext(Context);
   const [state, setState] = useImmediateState<UseMutationState<T>>({
     fetching: false,
@@ -34,7 +36,7 @@ export const useMutation = <T = any, V = object>(
       const request = createRequest(query, variables as any);
 
       return pipe(
-        client.executeMutation(request),
+        client.executeMutation(request, devtoolsContext),
         toPromise
       ).then(result => {
         const { data, error } = result;
@@ -42,7 +44,7 @@ export const useMutation = <T = any, V = object>(
         return result;
       });
     },
-    [client, query, setState]
+    [client, devtoolsContext, query, setState]
   );
 
   return [state, executeMutation];
diff --git a/src/hooks/useQuery.spec.ts b/src/hooks/useQuery.spec.ts
index e0db51e4..8875e357 100644
--- a/src/hooks/useQuery.spec.ts
+++ b/src/hooks/useQuery.spec.ts
@@ -90,9 +90,9 @@ describe('useQuery', () => {
         query: expect.any(Object),
         variables: mockVariables,
       },
-      {
+      expect.objectContaining({
         requestPolicy: undefined,
-      }
+      })
     );
   });
 
@@ -138,9 +138,9 @@ describe('useQuery', () => {
         query: expect.any(Object),
         variables: {},
       },
-      {
+      expect.objectContaining({
         requestPolicy: undefined,
-      }
+      })
     );
   });
 
@@ -168,9 +168,9 @@ describe('useQuery', () => {
         query: expect.any(Object),
         variables: newVariables,
       },
-      {
+      expect.objectContaining({
         requestPolicy: undefined,
-      }
+      })
     );
   });
 
@@ -211,9 +211,9 @@ describe('useQuery', () => {
         query: expect.any(Object),
         variables: mockVariables,
       },
-      {
+      expect.objectContaining({
         requestPolicy: 'cache-first',
-      }
+      })
     );
 
     rerender({
@@ -229,9 +229,9 @@ describe('useQuery', () => {
         query: expect.any(Object),
         variables: mockVariables,
       },
-      {
+      expect.objectContaining({
         requestPolicy: 'network-only',
-      }
+      })
     );
   });
 
diff --git a/src/hooks/useQuery.ts b/src/hooks/useQuery.ts
index 9b3a4108..cb25c6ed 100644
--- a/src/hooks/useQuery.ts
+++ b/src/hooks/useQuery.ts
@@ -4,6 +4,7 @@ import { pipe, subscribe } from 'wonka';
 import { Context } from '../context';
 import { OperationContext, RequestPolicy } from '../types';
 import { CombinedError, noop } from '../utils';
+import { useDevtoolsContext } from './useDevtoolsContext';
 import { useRequest } from './useRequest';
 import { useImmediateEffect } from './useImmediateEffect';
 import { useImmediateState } from './useImmediateState';
@@ -29,6 +30,7 @@ export type UseQueryResponse<T> = [
 export const useQuery = <T = any, V = object>(
   args: UseQueryArgs<V>
 ): UseQueryResponse<T> => {
+  const [devtoolsContext] = useDevtoolsContext();
   const unsubscribe = useRef(noop);
   const client = useContext(Context);
 
@@ -54,13 +56,14 @@ export const useQuery = <T = any, V = object>(
         client.executeQuery(request, {
           requestPolicy: args.requestPolicy,
           ...opts,
+          ...devtoolsContext,
         }),
         subscribe(({ data, error }) => {
           setState({ fetching: false, data, error });
         })
       );
     },
-    [args.requestPolicy, client, request, setState]
+    [args.requestPolicy, client, devtoolsContext, request, setState]
   );
 
   useImmediateEffect(() => {
@@ -71,7 +74,7 @@ export const useQuery = <T = any, V = object>(
     }
 
     executeQuery();
-    return () => unsubscribe.current();
+    return () => unsubscribe.current(); // eslint-disable-line
   }, [executeQuery, args.pause, setState]);
 
   return [state, executeQuery];
diff --git a/src/hooks/useSubscription.ts b/src/hooks/useSubscription.ts
index b5ca745d..42966481 100644
--- a/src/hooks/useSubscription.ts
+++ b/src/hooks/useSubscription.ts
@@ -3,6 +3,7 @@ import { useCallback, useContext, useEffect, useRef } from 'react';
 import { pipe, subscribe } from 'wonka';
 import { Context } from '../context';
 import { CombinedError, noop } from '../utils';
+import { useDevtoolsContext } from './useDevtoolsContext';
 import { useRequest } from './useRequest';
 import { useImmediateState } from './useImmediateState';
 
@@ -25,6 +26,7 @@ export const useSubscription = <T = any, R = T, V = object>(
   args: UseSubscriptionArgs<V>,
   handler?: SubscriptionHandler<T, R>
 ): UseSubscriptionResponse<R> => {
+  const [devtoolsContext] = useDevtoolsContext();
   const unsubscribe = useRef(noop);
   const client = useContext(Context);
 
@@ -42,7 +44,7 @@ export const useSubscription = <T = any, R = T, V = object>(
     unsubscribe.current();
 
     [unsubscribe.current] = pipe(
-      client.executeSubscription(request),
+      client.executeSubscription(request, devtoolsContext),
       subscribe(({ data, error }) => {
         setState(s => ({
           fetching: true,
@@ -51,14 +53,14 @@ export const useSubscription = <T = any, R = T, V = object>(
         }));
       })
     );
-  }, [client, handler, request, setState]);
+  }, [client, devtoolsContext, handler, request, setState]);
 
   // Trigger subscription on query change
   // We don't use useImmediateEffect here as we have no way of
   // unsubscribing from subscriptions during SSR
   useEffect(() => {
     executeSubscription();
-    return () => unsubscribe.current();
+    return () => unsubscribe.current(); // eslint-disable-line
   }, [executeSubscription]);
 
   return [state];
diff --git a/src/index.ts b/src/index.ts
index 5ce20da5..80a26fd7 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -4,4 +4,4 @@ export * from './components';
 export * from './exchanges';
 export * from './hooks';
 export * from './types';
-export * from './utils';
+export { CombinedError, createRequest } from './utils';
diff --git a/src/utils/index.ts b/src/utils/index.ts
index 99f7b9aa..6021f177 100644
--- a/src/utils/index.ts
+++ b/src/utils/index.ts
@@ -1,9 +1,9 @@
-export { CombinedError } from './error';
-export { getKeyForRequest } from './keyForQuery';
-export { createRequest } from './request';
+export * from './error';
+export * from './keyForQuery';
+export * from './request';
 export * from './ssr';
-export { formatDocument, collectTypesFromResponse } from './typenames';
-export { toSuspenseSource } from './toSuspenseSource';
+export * from './typenames';
+export * from './toSuspenseSource';
 
 export const noop = () => {
   /* noop */
diff --git a/src/utils/typenames.ts b/src/utils/typenames.ts
index 2fa9cc2b..384b594f 100755
--- a/src/utils/typenames.ts
+++ b/src/utils/typenames.ts
@@ -35,25 +35,36 @@ export const collectTypesFromResponse = (response: object) =>
 
 const formatNode = (
   n: FieldNode | InlineFragmentNode | OperationDefinitionNode
-) =>
-  n.selectionSet !== undefined && n.selectionSet.selections !== undefined
-    ? {
-        ...n,
-        selectionSet: {
-          ...n.selectionSet,
-          selections: [
-            ...n.selectionSet.selections,
-            {
-              kind: 'Field',
-              name: {
-                kind: 'Name',
-                value: '__typename',
-              },
-            },
-          ],
+) => {
+  if (n.selectionSet === undefined) {
+    return false;
+  }
+
+  if (
+    n.selectionSet.selections.some(
+      s => s.kind === 'Field' && s.name.value === '__typename'
+    )
+  ) {
+    return n;
+  }
+
+  return {
+    ...n,
+    selectionSet: {
+      ...n.selectionSet,
+      selections: [
+        ...n.selectionSet.selections,
+        {
+          kind: 'Field',
+          name: {
+            kind: 'Name',
+            value: '__typename',
+          },
         },
-      }
-    : false;
+      ],
+    },
+  };
+};
 
 export const formatDocument = (astNode: DocumentNode) =>
   visit(astNode, {


Test patch:
diff --git a/src/components/Query.test.tsx b/src/components/Query.test.tsx
index fb523c4f..801fa4a0 100644
--- a/src/components/Query.test.tsx
+++ b/src/components/Query.test.tsx
@@ -20,10 +20,8 @@ jest.mock('../client', () => {
 import * as React from 'react';
 import { cleanup, render } from '@testing-library/react';
 import { Query } from './Query';
-import { createClient } from '../client';
 
 // @ts-ignore
-const client = createClient();
 const query = '{ example }';
 const variables = {
   myVar: 1234,
diff --git a/src/components/Subscription.test.tsx b/src/components/Subscription.test.tsx
index bc6d7414..8537107e 100644
--- a/src/components/Subscription.test.tsx
+++ b/src/components/Subscription.test.tsx
@@ -20,10 +20,7 @@ jest.mock('../client', () => {
 import * as React from 'react';
 import { cleanup, render } from '@testing-library/react';
 import { Subscription } from './Subscription';
-import { createClient } from '../client';
 
-// @ts-ignore
-const client = createClient() as { executeSubscription: jest.Mock };
 const query = 'subscription Example { example }';
 
 describe('Subscription', () => {
diff --git a/src/hooks/useMutation.test.tsx b/src/hooks/useMutation.test.tsx
index 853136ab..5cffa0db 100644
--- a/src/hooks/useMutation.test.tsx
+++ b/src/hooks/useMutation.test.tsx
@@ -106,6 +106,16 @@ describe('on execute', () => {
       vars
     );
   });
+
+  it('calls executeMutation with source component info', () => {
+    renderer.create(<MutationUser {...props} />);
+    act(() => {
+      execute(vars);
+    });
+    expect(client.executeMutation.mock.calls[0][1]).toHaveProperty('devtools', {
+      source: 'MutationUser',
+    });
+  });
 });
 
 describe('on subscription update', () => {
diff --git a/src/hooks/useQuery.test.tsx b/src/hooks/useQuery.test.tsx
index 4cb69728..eb5cc4d3 100644
--- a/src/hooks/useQuery.test.tsx
+++ b/src/hooks/useQuery.test.tsx
@@ -83,9 +83,22 @@ describe('on initial useEffect', () => {
         query: expect.any(Object),
         variables: props.variables,
       },
-      {
+      expect.objectContaining({
         requestPolicy: undefined,
-      }
+      })
+    );
+  });
+
+  it('passes source component name to executeQuery', () => {
+    renderer.create(<QueryUser {...props} />);
+
+    expect(client.executeQuery).toBeCalledWith(
+      expect.any(Object),
+      expect.objectContaining({
+        devtools: {
+          source: 'QueryUser',
+        },
+      })
     );
   });
 });
diff --git a/src/hooks/useSubscription.test.tsx b/src/hooks/useSubscription.test.tsx
index fb66e637..3ab36f77 100644
--- a/src/hooks/useSubscription.test.tsx
+++ b/src/hooks/useSubscription.test.tsx
@@ -51,11 +51,22 @@ describe('on initial useEffect', () => {
 
   it('passes query to executeSubscription', () => {
     renderer.create(<SubscriptionUser q={query} />);
-    expect(client.executeSubscription).toBeCalledWith({
-      key: expect.any(Number),
-      query: expect.any(Object),
-      variables: {},
-    });
+    expect(client.executeSubscription).toBeCalledWith(
+      {
+        key: expect.any(Number),
+        query: expect.any(Object),
+        variables: {},
+      },
+      expect.any(Object)
+    );
+  });
+
+  it('passes source component info to executeSubscription', () => {
+    renderer.create(<SubscriptionUser q={query} />);
+    expect(client.executeSubscription).toBeCalledWith(
+      expect.any(Object),
+      expect.objectContaining({ devtools: { source: 'SubscriptionUser' } })
+    );
   });
 });
 
diff --git a/src/utils/typenames.test.ts b/src/utils/typenames.test.ts
index e8b95a83..aadf1403 100755
--- a/src/utils/typenames.test.ts
+++ b/src/utils/typenames.test.ts
@@ -7,15 +7,37 @@ const formatTypeNames = (query: string) => {
 };
 
 describe('formatTypeNames', () => {
-  it('should add typenames to a query string', () => {
-    expect(formatTypeNames(`{ todos { id } }`)).toBe(`{
-  todos {
-    id
-    __typename
-  }
-  __typename
-}
-`);
+  it('adds typenames to a query string', () => {
+    expect(formatTypeNames(`{ todos { id } }`)).toMatchInlineSnapshot(`
+      "{
+        todos {
+          id
+          __typename
+        }
+        __typename
+      }
+      "
+    `);
+  });
+
+  it('does not duplicate typenames', () => {
+    expect(
+      formatTypeNames(`{ 
+      todos { 
+        id
+      }
+      __typename
+    }`)
+    ).toMatchInlineSnapshot(`
+            "{
+              todos {
+                id
+                __typename
+              }
+              __typename
+            }
+            "
+        `);
   });
 });