Repository: FormidableLabs/urql (ts)

Issue:
createRequest implementation detail for comments (#)
urql version & exchanges: master

Basically [comment removal feature](https://github.com/FormidableLabs/urql/blob/1773e6ee509e514da9b34cb023d4d1b7a43cbe93/packages/core/src/utils/request.ts#L31) breaks the query hashing so the following test fails:

```ts
it('should not remove # from inputs', () => {
  const doc1 = `
    {
      test(name: "aaa#aaa")
    }
  `;
  const val1 = createRequest(doc1);
  expect(print(val1.query)).toBe(`{
  test(name: "aaa#aaa")
}
`);

  const doc2 = `
    {
      test(name: "aaa#bbb")
    }
  `;
  const val2 = createRequest(doc2);
  expect(print(val2.query)).toBe(`{
  test(name: "aaa#bbb")
}
`);
});

```

```bash
  ● should not remove # from inputs

    expect(received).toBe(expected) // Object.is equality

    - Expected  - 1
    + Received  + 1

      {
    -   test(name: "aaa#bbb")
    +   test(name: "aaa#aaa")
      }
      ↵

      112 |   `;
      113 |   const val2 = createRequest(doc2);
    > 114 |   expect(print(val2.query)).toBe(`{
          |                                                   ^
      115 |   test(name: "aaa#bbb")
      116 | }
      117 | `);

      at Object.<anonymous> (src/utils/request.test.ts:114:51)
```
Mutations not invalidating cache when sending a mutation with data containing a hashtag (#)
Hi,

This issue seems to be related to https://github.com/FormidableLabs/urql/issues/195

If I send this mutation the first time, everything is OK:

```
mutation updateItem {
    updateItem(input: {
        id: "path/to/1",
        name: "URQL example #1",
        comment:"Hello world"
    }) {
        item {
            id
        }
    }}
```

But if I then change only the name to _URQL example#2_ and send the mutation again, then the old mutation is sent.

The new mutation shows up if I console.log my mutation before calling my URQL client:
```
console.log(mutation) // This outputs the new mutation (changed name)
client
            .mutation(mutation)
            .toPromise()
            .then((response) => {
...
})
```


A new mutation is sent if:
- Another property is changed as well (`name: "URQL example #2", comment: "Hello github"`)
- The content before the hashtag is changed (`name: "Something before the hashtag#and after"`)


I have set up my client as follows:
```
const client = createClient({
    url: API_GRAPHQL_URL,
    requestPolicy: "network-only",
    exchanges: [
        devtoolsExchange,
        dedupExchange,
        fetchExchange,
    ],
    fetchOptions: () => {
        return {
            credentials: "include",
            headers: {},
            cache: "no-cache",
        }
    },
})
```

My temporary fix is to always change another property in the mutation (append a space to the "comment" in the above case).

I am using urql@2.1.3 on Typescript react@17.0.2.

Pull request description:
(core) - Prevent ignored characters from being sanitized in strings
Resolve #2290
Resolve #1960

## Summary

This updates our string sanitization to exclude replacing ignored characters inside of strings.
This is pretty crude but should stop us from accidentally messing up queries to the point where they don't produce valid hashes any longer.

Edit: Bundlesize will be affected in shared import by +59B.

## Set of changes

- Update `createRequest`'s stringification to exclude strings in sanitization
- Add appropriate test (previous test wasn't actually checking _our_ string sanitization)


Patch:
diff --git a/.changeset/serious-queens-suffer.md b/.changeset/serious-queens-suffer.md
new file mode 100644
index 00000000..b28ca80d
--- /dev/null
+++ b/.changeset/serious-queens-suffer.md
@@ -0,0 +1,5 @@
+---
+'@urql/core': patch
+---
+
+Prevent ignored characters in GraphQL queries from being replaced inside strings and block strings. Previously we accepted sanitizing strings via regular expressions causing duplicate hashes as acceptable, since it'd only be caused when a string wasn't extracted into variables. This is fixed now however.
diff --git a/packages/core/src/utils/request.ts b/packages/core/src/utils/request.ts
index 7fdb8c65..c824036c 100644
--- a/packages/core/src/utils/request.ts
+++ b/packages/core/src/utils/request.ts
@@ -21,6 +21,12 @@ export interface KeyedDocumentNode extends DocumentNode {
   __key: number;
 }
 
+const GRAPHQL_STRING_RE = /("{3}[\s\S]*"{3}|"(?:\\.|[^"])*")/g;
+const REPLACE_CHAR_RE = /([\s,]|#[^\n\r]+)+/g;
+
+const replaceOutsideStrings = (str: string, idx: number) =>
+  idx % 2 === 0 ? str.replace(REPLACE_CHAR_RE, ' ').trim() : str;
+
 export const stringifyDocument = (
   node: string | DefinitionNode | DocumentNode
 ): string => {
@@ -28,8 +34,9 @@ export const stringifyDocument = (
     ? (node.loc && node.loc.source.body) || print(node)
     : node
   )
-    .replace(/([\s,]|#[^\n\r]+)+/g, ' ')
-    .trim();
+    .split(GRAPHQL_STRING_RE)
+    .map(replaceOutsideStrings)
+    .join('');
 
   if (typeof node !== 'string') {
     const operationName = 'definitions' in node && getOperationName(node);


Test patch:
diff --git a/packages/core/src/utils/request.test.ts b/packages/core/src/utils/request.test.ts
index c1041cd4..b45444e2 100644
--- a/packages/core/src/utils/request.test.ts
+++ b/packages/core/src/utils/request.test.ts
@@ -1,6 +1,6 @@
 import { parse, print } from 'graphql';
 import { gql } from '../gql';
-import { createRequest } from './request';
+import { createRequest, stringifyDocument } from './request';
 
 jest.mock('./hash', () => ({
   hash: jest.requireActual('./hash').hash,
@@ -79,13 +79,50 @@ it('should return a valid query object with variables', () => {
   });
 });
 
-it('should remove comments', () => {
-  const doc = `
-    { #query
-      # broken
-      test
-    }
-  `;
-  const val = createRequest(doc);
-  expect(print(val.query)).toBe(`{\n  test\n}`);
+describe('stringifyDocument (internal API)', () => {
+  it('should remove comments', () => {
+    const doc = `
+      { #query
+        # broken
+        test
+      }
+    `;
+    expect(stringifyDocument(createRequest(doc).query)).toBe('{ test }');
+  });
+
+  it('should remove duplicate spaces', () => {
+    const doc = `
+      {
+        abc          ,, test
+      }
+    `;
+    expect(stringifyDocument(createRequest(doc).query)).toBe('{ abc test }');
+  });
+
+  it('should not sanitize within strings', () => {
+    const doc = `
+      {
+        field(arg: "test #1")
+      }
+    `;
+    expect(stringifyDocument(createRequest(doc).query)).toBe(
+      '{ field(arg:"test #1") }'
+    );
+  });
+
+  it('should not sanitize within block strings', () => {
+    const doc = `
+      {
+        field(
+          arg: """
+          hello
+          hello
+          """
+        )
+      }
+    `;
+    expect(stringifyDocument(createRequest(doc).query)).toBe(
+      '{ field(arg:"""\n  hello\n  hello\n  """) }'
+    );
+  });
 });