(core) - Refactor parts of core helpers for minor size savings (#658)

* Replace ciruclar items in stringifyVariables with null

* Remove immutable spread in formatDocument in typenames

* Refacto fetch/result

* Combine seen clause with null clause

* Add changeset

.changeset/empty-rabbits-eat.md

@@ -0,0 +1,5 @@
+---
+'@urql/core': patch
+---
+
+Refactor a couple of core helpers for minor bundlesize savings.

packages/core/src/exchanges/fetch.ts

@@ -12,16 +12,16 @@ interface Body {
 
 /** A default exchange for fetching GraphQL requests. */
 export const fetchExchange: Exchange = ({ forward }) => {
-  const isOperationFetchable = (operation: Operation) => {
-    const { operationName } = operation;
-    return operationName === 'query' || operationName === 'mutation';
-  };
-
   return ops$ => {
     const sharedOps$ = share(ops$);
     const fetchResults$ = pipe(
       sharedOps$,
-      filter(isOperationFetchable),
+      filter(operation => {
+        return (
+          operation.operationName === 'query' ||
+          operation.operationName === 'mutation'
+        );
+      }),
       mergeMap(operation => {
         const { key } = operation;
         const teardown$ = pipe(
@@ -42,7 +42,12 @@ export const fetchExchange: Exchange = ({ forward }) => {
 
     const forward$ = pipe(
       sharedOps$,
-      filter(op => !isOperationFetchable(op)),
+      filter(operation => {
+        return (
+          operation.operationName !== 'query' &&
+          operation.operationName !== 'mutation'
+        );
+      }),
       forward
     );
 
@@ -57,7 +62,7 @@ const getOperationName = (query: DocumentNode): string | null => {
     }
   );
 
-  return node !== undefined && node.name ? node.name.value : null;
+  return node ? node.name!.value : null;
 };
 
 const createFetchSource = (operation: Operation, shouldUseGet: boolean) => {

packages/core/src/utils/result.ts

@@ -15,9 +15,7 @@ export const makeResult = (
       })
     : undefined,
   extensions:
-    typeof result.extensions === 'object' && result.extensions !== null
-      ? result.extensions
-      : undefined,
+    (typeof result.extensions === 'object' && result.extensions) || undefined,
 });
 
 export const makeErrorResult = (

packages/core/src/utils/stringifyVariables.test.ts

@@ -21,12 +21,10 @@ it('stringifies scalars', () => {
   expect(stringifyVariables(1 / 0)).toBe('null');
 });
 
-it('throws for circular structures', () => {
-  expect(() => {
-    const x = { x: null } as any;
-    x.x = x;
-    stringifyVariables(x);
-  }).toThrow();
+it('returns null for circular structures', () => {
+  const x = { x: null } as any;
+  x.x = x;
+  expect(stringifyVariables(x)).toBe('{"x":null}');
 });
 
 it('stringifies dates correctly', () => {

packages/core/src/utils/stringifyVariables.ts

@@ -2,21 +2,14 @@ const seen = new Set();
 const cache = new WeakMap();
 
 const stringify = (x: any): string => {
-  if (x === undefined) {
-    return '';
-  } else if (typeof x == 'number') {
-    return isFinite(x) ? '' + x : 'null';
-  } else if (typeof x !== 'object') {
-    return JSON.stringify(x);
-  } else if (x === null) {
+  if (x === null || seen.has(x)) {
     return 'null';
+  } else if (typeof x !== 'object') {
+    return JSON.stringify(x) || '';
   } else if (x.toJSON) {
     return x.toJSON();
-  }
-
-  let out = '';
-  if (Array.isArray(x)) {
-    out = '[';
+  } else if (Array.isArray(x)) {
+    let out = '[';
     for (let i = 0, l = x.length; i < l; i++) {
       if (i > 0) out += ',';
       const value = stringify(x[i]);
@@ -25,8 +18,6 @@ const stringify = (x: any): string => {
 
     out += ']';
     return out;
-  } else if (seen.has(x)) {
-    throw new TypeError('Converting circular structure to JSON');
   }
 
   const keys = Object.keys(x).sort();
@@ -41,11 +32,11 @@ const stringify = (x: any): string => {
   }
 
   seen.add(x);
-  out = '{';
+  let out = '{';
   for (let i = 0, l = keys.length; i < l; i++) {
     const key = keys[i];
     const value = stringify(x[key]);
-    if (value.length !== 0) {
+    if (value) {
       if (out.length > 1) out += ',';
       out += stringify(key) + ':' + value;
     }

packages/core/src/utils/typenames.test.ts

@@ -7,6 +7,24 @@ const formatTypeNames = (query: string) => {
 };
 
 describe('formatTypeNames', () => {
+  it('creates a new instance when adding typenames', () => {
+    const doc = parse(`{ todos { id } }`) as any;
+    const newDoc = formatDocument(doc) as any;
+    expect(doc).not.toBe(newDoc);
+    expect(doc.definitions).not.toBe(newDoc.definitions);
+    expect(doc.definitions[0]).not.toBe(newDoc.definitions[0]);
+    expect(doc.definitions[0].selectionSet).not.toBe(
+      newDoc.definitions[0].selectionSet
+    );
+    expect(doc.definitions[0].selectionSet.selections).not.toBe(
+      newDoc.definitions[0].selectionSet.selections
+    );
+    // Here we're equal again:
+    expect(doc.definitions[0].selectionSet.selections[0]).toBe(
+      newDoc.definitions[0].selectionSet.selections[0]
+    );
+  });
+
   it('adds typenames to a query string', () => {
     expect(formatTypeNames(`{ todos { id } }`)).toMatchInlineSnapshot(`
       "{

packages/core/src/utils/typenames.ts

@@ -2,6 +2,8 @@ import {
   DocumentNode,
   FieldNode,
   InlineFragmentNode,
+  SelectionSetNode,
+  SelectionNode,
   Kind,
   visit,
 } from 'graphql';
@@ -18,13 +20,11 @@ const collectTypes = (obj: EntityLike | EntityLike[], types: string[] = []) => {
     });
   } else if (typeof obj === 'object' && obj !== null) {
     for (const key in obj) {
-      if (Object.prototype.hasOwnProperty.call(obj, key)) {
-        const val = obj[key];
-        if (key === '__typename' && typeof val === 'string') {
-          types.push(val);
-        } else if (typeof val === 'object' && val !== null) {
-          collectTypes(val, types);
-        }
+      const val = obj[key];
+      if (key === '__typename' && typeof val === 'string') {
+        types.push(val);
+      } else {
+        collectTypes(val, types);
       }
     }
   }
@@ -35,39 +35,33 @@ const collectTypes = (obj: EntityLike | EntityLike[], types: string[] = []) => {
 export const collectTypesFromResponse = (response: object) =>
   collectTypes(response as EntityLike).filter((v, i, a) => a.indexOf(v) === i);
 
-const formatNode = (n: FieldNode | InlineFragmentNode) => {
-  if (n.selectionSet === undefined) {
+const hasTypenameField = (set: SelectionSetNode) => {
+  return set.selections.some(node => {
+    return node.kind === Kind.FIELD && node.name.value === '__typename';
+  });
+};
+
+const formatNode = (node: FieldNode | InlineFragmentNode) => {
+  if (!node.selectionSet) {
     return false;
-  }
+  } else if (!hasTypenameField(node.selectionSet)) {
+    // NOTE: It's fine to mutate here as long as we return the node,
+    // which will instruct visit() to clone the AST upwards
+    (node.selectionSet.selections as SelectionNode[]).push({
+      kind: Kind.FIELD,
+      name: {
+        kind: Kind.NAME,
+        value: '__typename',
+      },
+    });
 
-  if (
-    n.selectionSet.selections.some(
-      s => s.kind === 'Field' && s.name.value === '__typename'
-    )
-  ) {
-    return n;
+    return node;
   }
-
-  return {
-    ...n,
-    selectionSet: {
-      ...n.selectionSet,
-      selections: [
-        ...n.selectionSet.selections,
-        {
-          kind: Kind.FIELD,
-          name: {
-            kind: Kind.NAME,
-            value: '__typename',
-          },
-        },
-      ],
-    },
-  };
 };
 
-export const formatDocument = (astNode: DocumentNode) =>
-  visit(astNode, {
+export const formatDocument = (node: DocumentNode) => {
+  return visit(node, {
     Field: formatNode,
     InlineFragment: formatNode,
   });
+};