Skip to content

Commit 1aca7ed

Browse files
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 <[email protected]> --------- Co-authored-by: Jerel Miller <[email protected]>
1 parent 60592e9 commit 1aca7ed

File tree

3 files changed

+71
-16
lines changed

3 files changed

+71
-16
lines changed

.changeset/early-pots-rule.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@apollo/client": patch
3+
---
4+
5+
`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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import type { DocumentNode } from "graphql";
21
import type * as ReactTypes from "react";
32

43
import type { ObservableQuery, OperationVariables } from "../../core/index.js";
54
import type { QueryDataOptions } from "../types/types.js";
5+
import { Trie } from "@wry/trie";
6+
import { canonicalStringify } from "../../cache/index.js";
67

78
// TODO: A vestigial interface from when hooks were implemented with utility
89
// classes, which should be deleted in the future.
@@ -16,11 +17,13 @@ type QueryInfo = {
1617
observable: ObservableQuery<any, any> | null;
1718
};
1819

19-
function makeDefaultQueryInfo(): QueryInfo {
20-
return {
20+
function makeQueryInfoTrie() {
21+
// these Tries are very short-lived, so we don't need to worry about making it
22+
// "weak" - it's easier to test and debug as a strong Trie.
23+
return new Trie<QueryInfo>(false, () => ({
2124
seen: false,
2225
observable: null,
23-
};
26+
}));
2427
}
2528

2629
export class RenderPromises {
@@ -31,13 +34,13 @@ export class RenderPromises {
3134
// objects. These QueryInfo objects are intended to survive through the whole
3235
// getMarkupFromTree process, whereas specific Query instances do not survive
3336
// beyond a single call to renderToStaticMarkup.
34-
private queryInfoTrie = new Map<DocumentNode, Map<string, QueryInfo>>();
37+
private queryInfoTrie = makeQueryInfoTrie();
3538

3639
private stopped = false;
3740
public stop() {
3841
if (!this.stopped) {
3942
this.queryPromises.clear();
40-
this.queryInfoTrie.clear();
43+
this.queryInfoTrie = makeQueryInfoTrie();
4144
this.stopped = true;
4245
}
4346
}
@@ -133,13 +136,9 @@ export class RenderPromises {
133136
private lookupQueryInfo<TData, TVariables extends OperationVariables>(
134137
props: QueryDataOptions<TData, TVariables>
135138
): QueryInfo {
136-
const { queryInfoTrie } = this;
137-
const { query, variables } = props;
138-
const varMap = queryInfoTrie.get(query) || new Map<string, QueryInfo>();
139-
if (!queryInfoTrie.has(query)) queryInfoTrie.set(query, varMap);
140-
const variablesString = JSON.stringify(variables);
141-
const info = varMap.get(variablesString) || makeDefaultQueryInfo();
142-
if (!varMap.has(variablesString)) varMap.set(variablesString, info);
143-
return info;
139+
return this.queryInfoTrie.lookup(
140+
props.query,
141+
canonicalStringify(props.variables)
142+
);
144143
}
145144
}

src/react/ssr/__tests__/useQuery.test.tsx

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@
22
import React from "react";
33
import { DocumentNode } from "graphql";
44
import gql from "graphql-tag";
5-
import { MockedProvider, mockSingleLink } from "../../../testing";
5+
import {
6+
MockedProvider,
7+
MockedResponse,
8+
mockSingleLink,
9+
} from "../../../testing";
610
import { ApolloClient } from "../../../core";
711
import { InMemoryCache } from "../../../cache";
8-
import { ApolloProvider } from "../../context";
12+
import { ApolloProvider, getApolloContext } from "../../context";
913
import { useApolloClient, useQuery } from "../../hooks";
1014
import { renderToStringWithData } from "..";
15+
import type { Trie } from "@wry/trie";
1116

1217
describe("useQuery Hook SSR", () => {
1318
const CAR_QUERY: DocumentNode = gql`
@@ -333,4 +338,50 @@ describe("useQuery Hook SSR", () => {
333338
expect(cache.extract()).toMatchSnapshot();
334339
});
335340
});
341+
342+
it("should deduplicate `variables` with identical content, but different order", async () => {
343+
const mocks: MockedResponse[] = [
344+
{
345+
request: {
346+
query: CAR_QUERY,
347+
variables: { foo: "a", bar: 1 },
348+
},
349+
result: { data: CAR_RESULT_DATA },
350+
maxUsageCount: 1,
351+
},
352+
];
353+
354+
let trie: Trie<any> | undefined;
355+
const Component = ({
356+
variables,
357+
}: {
358+
variables: { foo: string; bar: number };
359+
}) => {
360+
const { loading, data } = useQuery(CAR_QUERY, { variables, ssr: true });
361+
trie ||=
362+
React.useContext(getApolloContext()).renderPromises!["queryInfoTrie"];
363+
if (!loading) {
364+
expect(data).toEqual(CAR_RESULT_DATA);
365+
const { make, model, vin } = data.cars[0];
366+
return (
367+
<div>
368+
{make}, {model}, {vin}
369+
</div>
370+
);
371+
}
372+
return null;
373+
};
374+
375+
await renderToStringWithData(
376+
<MockedProvider mocks={mocks}>
377+
<>
378+
<Component variables={{ foo: "a", bar: 1 }} />
379+
<Component variables={{ bar: 1, foo: "a" }} />
380+
</>
381+
</MockedProvider>
382+
);
383+
expect(
384+
Array.from(trie!["getChildTrie"](CAR_QUERY)["strong"].keys())
385+
).toEqual(['{"bar":1,"foo":"a"}']);
386+
});
336387
});

0 commit comments

Comments
 (0)