RenderPromises: use canonicalStringify to serialize data, use Trie

.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.

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)
+    );
   }
 }

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"}']);
+  });
 });