Repository: apollographql/apollo-client

Problem statement:
useQuery fails to resolve ssr renderPromise due to JSON.stringify reordering
### Issue Description

We found a bug in production where the [`lookupQueryInfo()`](https://github.com/apollographql/apollo-client/blob/main/src/react/ssr/RenderPromises.ts#L140) method would consistently not render a SSR component after calling `renderToStringWithData`. 

Upon investigation, we found that the query was being executed and the result should have been available. However, a second `QueryInfo` would be registered instead of recovering the resolved one. This is due to the `JSON.stringify` method switching the order of attributes when looking up the Map (Check screenshots).

---
<img width="1032" alt="Screenshot 2024-04-19 at 4 27 10 PM" src="https://github.com/apollographql/apollo-client/assets/11381625/ecbd4f7c-c2ec-46e5-85ef-8243105a3e1d">


<img width="1189" alt="Screenshot 2024-04-19 at 4 28 15 PM" src="https://github.com/apollographql/apollo-client/assets/11381625/d4813464-8c28-4d83-bda4-9be0faa58145">

---
I noticed the utilities package already provides a [`canonicalStringify`](https://github.com/apollographql/apollo-client/blob/main/src/utilities/common/canonicalStringify.ts) which would fix this issue.




### Link to Reproduction

https://github.com/apollographql/apollo-client/blob/main/src/react/ssr/RenderPromises.ts#L140

### Reproduction Steps

_No response_

### `@apollo/client` version

3.8.3


Hints:
That's interesting - are you passing different variables in different situations here?    
I don't think that Apollo Client ever touches variables in a writing manner, and JSON.stringify would always stringify things in the object order.

That said, using `canonicalStringify` absolutely makes sense here. I'll prepare a fix.
Amazing, @phryneas. Thank you for the fast response. 

That's exactly what I found weird about it. We don't pass different variables to it (although I guess the react re-rendering recreates the objects/variables that are passed when the functional component gets re-executed). I tried getting `JSON.stringify` to reproduce the issue in isolation but couldn't.
I am also willing to make the contribution if you are short on time. Let me know.

Patch:
diff --git a/src/react/ssr/RenderPromises.ts b/src/react/ssr/RenderPromises.ts
--- a/src/react/ssr/RenderPromises.ts
+++ b/src/react/ssr/RenderPromises.ts
@@ -1,8 +1,9 @@
-import type { DocumentNode } from "graphql";
 import type * as ReactTypes from "react";
 
 import type { ObservableQuery, OperationVariables } from "../../core/index.js";
 import type { QueryDataOptions } from "../types/types.js";
+import { Trie } from "@wry/trie";
+import { canonicalStringify } from "../../cache/index.js";
 
 // TODO: A vestigial interface from when hooks were implemented with utility
 // classes, which should be deleted in the future.
@@ -16,11 +17,13 @@ type QueryInfo = {
   observable: ObservableQuery<any, any> | null;
 };
 
-function makeDefaultQueryInfo(): QueryInfo {
-  return {
+function makeQueryInfoTrie() {
+  // these Tries are very short-lived, so we don't need to worry about making it
+  // "weak" - it's easier to test and debug as a strong Trie.
+  return new Trie<QueryInfo>(false, () => ({
     seen: false,
     observable: null,
-  };
+  }));
 }
 
 export class RenderPromises {
@@ -31,13 +34,13 @@ export class RenderPromises {
   // objects. These QueryInfo objects are intended to survive through the whole
   // getMarkupFromTree process, whereas specific Query instances do not survive
   // beyond a single call to renderToStaticMarkup.
-  private queryInfoTrie = new Map<DocumentNode, Map<string, QueryInfo>>();
+  private queryInfoTrie = makeQueryInfoTrie();
 
   private stopped = false;
   public stop() {
     if (!this.stopped) {
       this.queryPromises.clear();
-      this.queryInfoTrie.clear();
+      this.queryInfoTrie = makeQueryInfoTrie();
       this.stopped = true;
     }
   }
@@ -133,13 +136,9 @@ export class RenderPromises {
   private lookupQueryInfo<TData, TVariables extends OperationVariables>(
     props: QueryDataOptions<TData, TVariables>
   ): QueryInfo {
-    const { queryInfoTrie } = this;
-    const { query, variables } = props;
-    const varMap = queryInfoTrie.get(query) || new Map<string, QueryInfo>();
-    if (!queryInfoTrie.has(query)) queryInfoTrie.set(query, varMap);
-    const variablesString = JSON.stringify(variables);
-    const info = varMap.get(variablesString) || makeDefaultQueryInfo();
-    if (!varMap.has(variablesString)) varMap.set(variablesString, info);
-    return info;
+    return this.queryInfoTrie.lookup(
+      props.query,
+      canonicalStringify(props.variables)
+    );
   }
 }


Test patch:
diff --git a/src/react/ssr/__tests__/useQuery.test.tsx b/src/react/ssr/__tests__/useQuery.test.tsx
--- a/src/react/ssr/__tests__/useQuery.test.tsx
+++ b/src/react/ssr/__tests__/useQuery.test.tsx
@@ -2,12 +2,17 @@
 import React from "react";
 import { DocumentNode } from "graphql";
 import gql from "graphql-tag";
-import { MockedProvider, mockSingleLink } from "../../../testing";
+import {
+  MockedProvider,
+  MockedResponse,
+  mockSingleLink,
+} from "../../../testing";
 import { ApolloClient } from "../../../core";
 import { InMemoryCache } from "../../../cache";
-import { ApolloProvider } from "../../context";
+import { ApolloProvider, getApolloContext } from "../../context";
 import { useApolloClient, useQuery } from "../../hooks";
 import { renderToStringWithData } from "..";
+import type { Trie } from "@wry/trie";
 
 describe("useQuery Hook SSR", () => {
   const CAR_QUERY: DocumentNode = gql`
@@ -333,4 +338,50 @@ describe("useQuery Hook SSR", () => {
       expect(cache.extract()).toMatchSnapshot();
     });
   });
+
+  it("should deduplicate `variables` with identical content, but different order", async () => {
+    const mocks: MockedResponse[] = [
+      {
+        request: {
+          query: CAR_QUERY,
+          variables: { foo: "a", bar: 1 },
+        },
+        result: { data: CAR_RESULT_DATA },
+        maxUsageCount: 1,
+      },
+    ];
+
+    let trie: Trie<any> | undefined;
+    const Component = ({
+      variables,
+    }: {
+      variables: { foo: string; bar: number };
+    }) => {
+      const { loading, data } = useQuery(CAR_QUERY, { variables, ssr: true });
+      trie ||=
+        React.useContext(getApolloContext()).renderPromises!["queryInfoTrie"];
+      if (!loading) {
+        expect(data).toEqual(CAR_RESULT_DATA);
+        const { make, model, vin } = data.cars[0];
+        return (
+          <div>
+            {make}, {model}, {vin}
+          </div>
+        );
+      }
+      return null;
+    };
+
+    await renderToStringWithData(
+      <MockedProvider mocks={mocks}>
+        <>
+          <Component variables={{ foo: "a", bar: 1 }} />
+          <Component variables={{ bar: 1, foo: "a" }} />
+        </>
+      </MockedProvider>
+    );
+    expect(
+      Array.from(trie!["getChildTrie"](CAR_QUERY)["strong"].keys())
+    ).toEqual(['{"bar":1,"foo":"a"}']);
+  });
 });