Refactor createRequest parsing and hashing (#383)

* Refactor createRequest parsing and hashing

Co-authored-by: Jovi De Croock <decroockjovi@gmail.com>

* Revert change to request return statement

* Remove graphql-tag dependency

* Fix consistent hashing between DocumentNodes and strings

* Fix copy-paste issue in normalizer

package.json

@@ -135,8 +135,6 @@
   },
   "dependencies": {
     "fast-json-stable-stringify": "^2.0.0",
-    "graphql-tag": "^2.10.1",
-    "prop-types": "^15.6.0",
     "wonka": "^3.2.0"
   }
 }

src/client.test.ts

@@ -3,11 +3,6 @@ import gql from 'graphql-tag';
 
 /** NOTE: Testing in this file is designed to test both the client and it's interaction with default Exchanges */
 
-jest.mock('./utils/keyForQuery', () => ({
-  getKeyForQuery: () => 123,
-  getKeyForRequest: () => 123,
-}));
-
 import { map, pipe, subscribe, tap } from 'wonka';
 import { createClient } from './client';
 

src/utils/hash.ts

@@ -0,0 +1,14 @@
+// When we have separate strings it's useful to run a progressive
+// version of djb2 where we pretend that we're still looping over
+// the same string
+export const phash = (h: number, x: string): number => {
+  h = h | 0;
+  for (let i = 0, l = x.length | 0; i < l; i++) {
+    h = (h << 5) + h + x.charCodeAt(i);
+  }
+
+  return h;
+};
+
+// This is a djb2 hashing function
+export const hash = (x: string): number => phash(5381 | 0, x) >>> 0;

src/utils/index.ts

@@ -1,5 +1,4 @@
 export * from './error';
-export * from './keyForQuery';
 export * from './request';
 export * from './typenames';
 export * from './toSuspenseSource';

src/utils/request.test.ts

@@ -1,21 +1,54 @@
-import { print } from 'graphql';
+import { parse, print } from 'graphql';
 import gql from 'graphql-tag';
 import { createRequest } from './request';
 
-const doc = print(
-  gql`
+it('should hash identical queries identically', () => {
+  const reqA = createRequest('{ test }');
+  const reqB = createRequest('{ test }');
+  expect(reqA.key).toBe(reqB.key);
+});
+
+it('should hash identical DocumentNodes identically', () => {
+  const reqA = createRequest(parse('{ testB }'));
+  const reqB = createRequest(parse('{ testB }'));
+  expect(reqA.key).toBe(reqB.key);
+  expect(reqA.query).toBe(reqB.query);
+});
+
+it('should use the hash from a key if available', () => {
+  const doc = parse('{ testC }');
+  (doc as any).__key = 1234;
+  const req = createRequest(doc);
+  expect(req.key).toBe(1234);
+});
+
+it('should hash DocumentNodes and strings identically', () => {
+  const docA = parse('{ field }');
+  const docB = print(docA).replace(/\s/g, ' ');
+  const reqA = createRequest(docA);
+  const reqB = createRequest(docB);
+  expect(reqA.key).toBe(reqB.key);
+  expect(reqA.query).toBe(reqB.query);
+});
+
+it('should hash graphql-tag documents correctly', () => {
+  const doc = gql`
     {
-      todos {
-        id
-      }
+      testD
     }
-  `
-);
+  `;
+  createRequest(doc);
+  expect((doc as any).__key).not.toBe(undefined);
+});
 
 it('should return a valid query object', () => {
+  const doc = gql`
+    {
+      testE
+    }
+  `;
   const val = createRequest(doc);
 
-  expect(print(val.query)).toBe(doc);
   expect(val).toMatchObject({
     key: expect.any(Number),
     query: expect.any(Object),
@@ -24,6 +57,13 @@ it('should return a valid query object', () => {
 });
 
 it('should return a valid query object with variables', () => {
+  const doc = print(
+    gql`
+      {
+        testF
+      }
+    `
+  );
   const val = createRequest(doc, { test: 5 });
 
   expect(print(val.query)).toBe(doc);

src/utils/request.ts

@@ -1,16 +1,39 @@
-import { DocumentNode } from 'graphql';
-import gql from 'graphql-tag';
-import { getKeyForRequest } from './keyForQuery';
+import { DocumentNode, parse, print } from 'graphql';
+import { hash, phash } from './hash';
+import stringify from 'fast-json-stable-stringify';
 import { GraphQLRequest, Operation, OperationContext } from '../types';
 
+interface Documents {
+  [key: number]: DocumentNode;
+}
+
+const hashQuery = (q: string): number => hash(q.replace(/[\s,]+/g, ' ').trim());
+
+const docs: Documents = Object.create(null);
+const keyProp = '__key';
+
 export const createRequest = (
   q: string | DocumentNode,
   vars?: object
 ): GraphQLRequest => {
-  const query = typeof q === 'string' ? gql([q]) : q;
+  let key: number;
+  let query: DocumentNode;
+  if (typeof q === 'string') {
+    key = hashQuery(q);
+    query = docs[key] !== undefined ? docs[key] : parse(q);
+  } else if ((q as any)[keyProp] !== undefined) {
+    key = (q as any)[keyProp];
+    query = q;
+  } else {
+    key = hashQuery(print(q));
+    query = docs[key] !== undefined ? docs[key] : q;
+  }
+
+  docs[key] = query;
+  (query as any)[keyProp] = key;
 
   return {
-    key: getKeyForRequest(query, vars),
+    key: vars ? phash(key, stringify(vars)) >>> 0 : key,
     query,
     variables: vars || {},
   };

yarn.lock

@@ -4816,15 +4816,6 @@ prop-types-exact@^1.2.0:
     object.assign "^4.1.0"
     reflect.ownkeys "^0.2.0"
 
-prop-types@^15.6.0, prop-types@^15.7.2:
-  version "15.7.2"
-  resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.7.2.tgz#52c41e75b8c87e72b9d9360e0206b99dcbffa6c5"
-  integrity sha512-8QQikdH7//R2vurIJSutZ1smHYTcLpRWEOlHnzcWHmBYrOGUysKwSsrC89BCiFj3CbrfJ/nXFdJepOVrY1GCHQ==
-  dependencies:
-    loose-envify "^1.4.0"
-    object-assign "^4.1.1"
-    react-is "^16.8.1"
-
 prop-types@^15.6.2:
   version "15.6.2"
   resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.6.2.tgz#05d5ca77b4453e985d60fc7ff8c859094a497102"
@@ -4833,6 +4824,15 @@ prop-types@^15.6.2:
     loose-envify "^1.3.1"
     object-assign "^4.1.1"
 
+prop-types@^15.7.2:
+  version "15.7.2"
+  resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.7.2.tgz#52c41e75b8c87e72b9d9360e0206b99dcbffa6c5"
+  integrity sha512-8QQikdH7//R2vurIJSutZ1smHYTcLpRWEOlHnzcWHmBYrOGUysKwSsrC89BCiFj3CbrfJ/nXFdJepOVrY1GCHQ==
+  dependencies:
+    loose-envify "^1.4.0"
+    object-assign "^4.1.1"
+    react-is "^16.8.1"
+
 psl@^1.1.24:
   version "1.1.32"
   resolved "https://registry.yarnpkg.com/psl/-/psl-1.1.32.tgz#3f132717cf2f9c169724b2b6caf373cf694198db"