From 1aca7ed5a3accf2303ccdf9b3dece7278f03ad62 Mon Sep 17 00:00:00 2001
From: Lenz Weber-Tronic <lorenz.weber-tronic@apollographql.com>
Date: Wed, 24 Apr 2024 20:30:37 +0200
Subject: [PATCH] `RenderPromises`: use `canonicalStringify` to serialize data,
 use `Trie` (#11799)

* `RenderPromises`: use `canonicalStringify` to serialize data, use `Trie`
fixes #11798
This ensures that queries with variables of equal contents, but different order, will be handled the same way during `renderToStringWithData` SSR.
It also replaces a hand-written Trie implementation with the `Trie` dependency.

* Update .changeset/early-pots-rule.md

Co-authored-by: Jerel Miller <jerelmiller@gmail.com>

---------

Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
---
 .changeset/early-pots-rule.md             |  5 +++
 src/react/ssr/RenderPromises.ts           | 27 ++++++-----
 src/react/ssr/__tests__/useQuery.test.tsx | 55 ++++++++++++++++++++++-
 3 files changed, 71 insertions(+), 16 deletions(-)
 create mode 100644 .changeset/early-pots-rule.md

diff --git a/.changeset/early-pots-rule.md b/.changeset/early-pots-rule.md
new file mode 100644
index 00000000000..3182add0cc2
--- /dev/null
+++ b/.changeset/early-pots-rule.md
@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+`RenderPromises`: use `canonicalStringify` to serialize `variables` to ensure query deduplication is properly applied even when `variables` are specified in a different order.
diff --git a/src/react/ssr/RenderPromises.ts b/src/react/ssr/RenderPromises.ts
index f51bcc93aca..44ca99519b7 100644
--- 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)
+    );
   }
 }
diff --git a/src/react/ssr/__tests__/useQuery.test.tsx b/src/react/ssr/__tests__/useQuery.test.tsx
index 3810f568575..02c711b9eba 100644
--- 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"}']);
+  });
 });