Skip to content

Commit 1a2b98e

Browse files
jonkaftonChristopherChudzicki
jonkafton
and
ChristopherChudzicki
authored
Mechanism to sync server prefetch with client API calls (#1798)
* Warn on API calls during initial render not prefetched * Full prefetch for homepage (commented) * Prefetch utility * Check for queries prefetched that are not needed during render and warn * No need to stringify * Replace useQuery overrides with decoupled cache check (wip) * Observer count for unnecessary prefetch warnings * Remove useQuery override * Test prefetch warnings * Remove inadvertent/unnecessary diff * Remove comments * Remove comment * Update frontends/api/src/ssr/usePrefetchWarnings.test.ts Co-authored-by: Chris Chudzicki <[email protected]> * Remove comment as no longer true * Less specific object assertion --------- Co-authored-by: Chris Chudzicki <[email protected]>
1 parent 41d284d commit 1a2b98e

File tree

13 files changed

+300
-30
lines changed

13 files changed

+300
-30
lines changed

‎frontends/api/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"./v1": "./src/generated/v1/api.ts",
1111
"./hooks/*": "./src/hooks/*/index.ts",
1212
"./constants": "./src/common/constants.ts",
13+
"./ssr/*": "./src/ssr/*.ts",
1314
"./test-utils/factories": "./src/test-utils/factories/index.ts",
1415
"./test-utils": "./src/test-utils/index.ts"
1516
},

‎frontends/api/src/hooks/channels/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const useChannelDetail = (channelType: string, channelName: string) => {
2727
...channels.detailByType(channelType, channelName),
2828
})
2929
}
30+
3031
const useChannelCounts = (channelType: string) => {
3132
return useQuery({
3233
...channels.countsByType(channelType),
@@ -54,4 +55,5 @@ export {
5455
useChannelsList,
5556
useChannelPartialUpdate,
5657
useChannelCounts,
58+
channels as channelsKeyFactory,
5759
}

‎frontends/api/src/hooks/learningResources/index.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -500,13 +500,6 @@ const useSchoolsList = () => {
500500
return useQuery(learningResources.schools())
501501
}
502502

503-
/*
504-
* Not intended to be imported except for special cases.
505-
* It's used in the ResourceCarousel to dynamically build a single useQueries hook
506-
* from config because a React component cannot conditionally call hooks during renders.
507-
*/
508-
export { default as learningResourcesKeyFactory } from "./keyFactory"
509-
510503
export {
511504
useLearningResourcesList,
512505
useFeaturedLearningResourcesList,
@@ -538,4 +531,5 @@ export {
538531
useListItemMove,
539532
usePlatformsList,
540533
useSchoolsList,
534+
learningResources as learningResourcesKeyFactory,
541535
}

‎frontends/api/src/hooks/newsEvents/index.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,9 @@ const useNewsEventsDetail = (id: number) => {
1515
return useQuery(newsEvents.detail(id))
1616
}
1717

18-
export { useNewsEventsList, useNewsEventsDetail, NewsEventsListFeedTypeEnum }
18+
export {
19+
useNewsEventsList,
20+
useNewsEventsDetail,
21+
NewsEventsListFeedTypeEnum,
22+
newsEvents as newsEventsKeyFactory,
23+
}

‎frontends/api/src/hooks/testimonials/index.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,8 @@ const useTestimonialDetail = (id: number | undefined) => {
2323
})
2424
}
2525

26-
export { useTestimonialDetail, useTestimonialList }
26+
export {
27+
useTestimonialDetail,
28+
useTestimonialList,
29+
testimonials as testimonialsKeyFactory,
30+
}

‎frontends/api/src/hooks/widget_lists/index.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"
33
import { widgetListsApi } from "../../clients"
44
import widgetLists from "./keyFactory"
55
import { WidgetInstance } from "api/v0"
6+
67
/**
7-
* Query is diabled if id is undefined.
8+
* Query is disabled if id is undefined.
89
*/
910
const useWidgetList = (id: number | undefined) => {
1011
return useQuery({

‎frontends/api/src/ssr/prefetch.ts

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { QueryClient, dehydrate } from "@tanstack/react-query"
2+
import type { Query } from "@tanstack/react-query"
3+
4+
// Utility to avoid repetition in server components
5+
export const prefetch = async (queries: (Query | unknown)[]) => {
6+
const queryClient = new QueryClient()
7+
8+
await Promise.all(
9+
queries.map((query) => queryClient.prefetchQuery(query as Query)),
10+
)
11+
12+
return dehydrate(queryClient)
13+
}

‎frontends/api/src/ssr/usePrefetchWarnings.test.ts

+118
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { renderHook } from "@testing-library/react"
2+
import { useQuery } from "@tanstack/react-query"
3+
import { usePrefetchWarnings } from "./usePrefetchWarnings"
4+
import { setupReactQueryTest } from "../hooks/test-utils"
5+
import { urls, factories, setMockResponse } from "../test-utils"
6+
import {
7+
learningResourcesKeyFactory,
8+
useLearningResourcesDetail,
9+
} from "../hooks/learningResources"
10+
11+
jest.mock("./usePrefetchWarnings", () => {
12+
const originalModule = jest.requireActual("./usePrefetchWarnings")
13+
return {
14+
...originalModule,
15+
logQueries: jest.fn(),
16+
}
17+
})
18+
19+
describe("SSR prefetch warnings", () => {
20+
beforeEach(() => {
21+
jest.spyOn(console, "info").mockImplementation(() => {})
22+
jest.spyOn(console, "table").mockImplementation(() => {})
23+
})
24+
25+
it("Warns if a query is requested on the client that has not been prefetched", async () => {
26+
const { wrapper, queryClient } = setupReactQueryTest()
27+
28+
const data = factories.learningResources.resource()
29+
setMockResponse.get(urls.learningResources.details({ id: 1 }), data)
30+
31+
renderHook(() => useLearningResourcesDetail(1), { wrapper })
32+
33+
renderHook(usePrefetchWarnings, {
34+
wrapper,
35+
initialProps: { queryClient },
36+
})
37+
38+
expect(console.info).toHaveBeenCalledWith(
39+
"The following queries were requested in first render but not prefetched.",
40+
"If these queries are user-specific, they cannot be prefetched - responses are cached on public CDN.",
41+
"Otherwise, consider fetching on the server with prefetch:",
42+
)
43+
expect(console.table).toHaveBeenCalledWith(
44+
[
45+
expect.objectContaining({
46+
disabled: false,
47+
initialStatus: "loading",
48+
key: learningResourcesKeyFactory.detail(1).queryKey,
49+
observerCount: 1,
50+
}),
51+
],
52+
["hash", "initialStatus", "status", "observerCount", "disabled"],
53+
)
54+
})
55+
56+
it("Ignores exempted queries requested on the client that have not been prefetched", async () => {
57+
const { wrapper, queryClient } = setupReactQueryTest()
58+
59+
const data = factories.learningResources.resource()
60+
setMockResponse.get(urls.learningResources.details({ id: 1 }), data)
61+
62+
renderHook(() => useLearningResourcesDetail(1), { wrapper })
63+
64+
renderHook(usePrefetchWarnings, {
65+
wrapper,
66+
initialProps: {
67+
queryClient,
68+
exemptions: [learningResourcesKeyFactory.detail(1).queryKey],
69+
},
70+
})
71+
72+
expect(console.info).not.toHaveBeenCalled()
73+
expect(console.table).not.toHaveBeenCalled()
74+
})
75+
76+
it("Warns for queries prefetched on the server but not requested on the client", async () => {
77+
const { wrapper, queryClient } = setupReactQueryTest()
78+
79+
const data = factories.learningResources.resource()
80+
setMockResponse.get(urls.learningResources.details({ id: 1 }), data)
81+
82+
// Emulate server prefetch
83+
const { unmount } = renderHook(
84+
() =>
85+
useQuery({
86+
...learningResourcesKeyFactory.detail(1),
87+
initialData: data,
88+
}),
89+
{ wrapper },
90+
)
91+
92+
// Removes observer
93+
unmount()
94+
95+
renderHook(usePrefetchWarnings, {
96+
wrapper,
97+
initialProps: { queryClient },
98+
})
99+
100+
expect(console.info).toHaveBeenCalledWith(
101+
"The following queries were prefetched on the server but not accessed during initial render.",
102+
"If these queries are no longer in use they should removed from prefetch:",
103+
)
104+
expect(console.table).toHaveBeenCalledWith(
105+
[
106+
{
107+
disabled: false,
108+
hash: JSON.stringify(learningResourcesKeyFactory.detail(1).queryKey),
109+
initialStatus: "success",
110+
key: learningResourcesKeyFactory.detail(1).queryKey,
111+
observerCount: 0,
112+
status: "success",
113+
},
114+
],
115+
["hash", "initialStatus", "status", "observerCount", "disabled"],
116+
)
117+
})
118+
})

‎frontends/api/src/ssr/usePrefetchWarnings.ts

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { useEffect } from "react"
2+
import type { Query, QueryClient, QueryKey } from "@tanstack/react-query"
3+
4+
const logQueries = (...args: [...string[], Query[]]) => {
5+
const queries = args.pop() as Query[]
6+
console.info(...args)
7+
console.table(
8+
queries.map((query) => ({
9+
key: query.queryKey,
10+
hash: query.queryHash,
11+
disabled: query.isDisabled(),
12+
initialStatus: query.initialState.status,
13+
status: query.state.status,
14+
observerCount: query.getObserversCount(),
15+
})),
16+
["hash", "initialStatus", "status", "observerCount", "disabled"],
17+
)
18+
}
19+
20+
const PREFETCH_EXEMPT_QUERIES = [["userMe"]]
21+
22+
/**
23+
* Call this as high as possible in render tree to detect query usage on
24+
* first render.
25+
*/
26+
export const usePrefetchWarnings = ({
27+
queryClient,
28+
exemptions = [],
29+
}: {
30+
queryClient: QueryClient
31+
/**
32+
* A list of query keys that should be exempted.
33+
*
34+
* NOTE: This uses react-query's hierarchical key matching, so exempting
35+
* ["a", { x: 1 }] will exempt
36+
* - ["a", { x: 1 }]
37+
* - ["a", { x: 1, y: 2 }]
38+
* - ["a", { x: 1, y: 2 }, ...any_other_entries]
39+
*/
40+
exemptions?: QueryKey[]
41+
}) => {
42+
/**
43+
* NOTE: React renders components top-down, but effects run bottom-up, so
44+
* this effect will run after all child effects.
45+
*/
46+
useEffect(
47+
() => {
48+
if (process.env.NODE_ENV === "production") {
49+
return
50+
}
51+
52+
const cache = queryClient.getQueryCache()
53+
const queries = cache.getAll()
54+
55+
const exempted = [...exemptions, ...PREFETCH_EXEMPT_QUERIES].map((key) =>
56+
cache.find(key),
57+
)
58+
59+
const potentialPrefetches = queries.filter(
60+
(query) =>
61+
!exempted.includes(query) &&
62+
query.initialState.status !== "success" &&
63+
!query.isDisabled(),
64+
)
65+
66+
if (potentialPrefetches.length > 0) {
67+
logQueries(
68+
"The following queries were requested in first render but not prefetched.",
69+
"If these queries are user-specific, they cannot be prefetched - responses are cached on public CDN.",
70+
"Otherwise, consider fetching on the server with prefetch:",
71+
potentialPrefetches,
72+
)
73+
}
74+
75+
const unusedPrefetches = queries.filter(
76+
(query) =>
77+
!exempted.includes(query) &&
78+
query.initialState.status === "success" &&
79+
query.getObserversCount() === 0 &&
80+
!query.isDisabled(),
81+
)
82+
83+
if (unusedPrefetches.length > 0) {
84+
logQueries(
85+
"The following queries were prefetched on the server but not accessed during initial render.",
86+
"If these queries are no longer in use they should removed from prefetch:",
87+
unusedPrefetches,
88+
)
89+
}
90+
},
91+
// We only want to run this on initial render.
92+
// (Aside: queryClient should be a singleton anyway)
93+
// eslint-disable-next-line react-hooks/exhaustive-deps
94+
[],
95+
)
96+
}

‎frontends/main/src/app-pages/HomePage/HomePage.tsx

+11-15
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"use client"
22

3-
import React, { Suspense } from "react"
3+
import React from "react"
44
import { Container, styled, theme } from "ol-components"
55
import HeroSearch from "@/page-components/HeroSearch/HeroSearch"
66
import BrowseTopicsSection from "./BrowseTopicsSection"
@@ -52,25 +52,21 @@ const HomePage: React.FC = () => {
5252
<StyledContainer>
5353
<HeroSearch />
5454
<section>
55-
<Suspense>
56-
<FeaturedCoursesCarousel
57-
titleComponent="h2"
58-
title="Featured Courses"
59-
config={carousels.FEATURED_RESOURCES_CAROUSEL}
60-
/>
61-
</Suspense>
55+
<FeaturedCoursesCarousel
56+
titleComponent="h2"
57+
title="Featured Courses"
58+
config={carousels.FEATURED_RESOURCES_CAROUSEL}
59+
/>
6260
</section>
6361
</StyledContainer>
6462
</FullWidthBackground>
6563
<PersonalizeSection />
6664
<Container component="section">
67-
<Suspense>
68-
<MediaCarousel
69-
titleComponent="h2"
70-
title="Media"
71-
config={carousels.MEDIA_CAROUSEL}
72-
/>
73-
</Suspense>
65+
<MediaCarousel
66+
titleComponent="h2"
67+
title="Media"
68+
config={carousels.MEDIA_CAROUSEL}
69+
/>
7470
</Container>
7571
<BrowseTopicsSection />
7672
<TestimonialsSection />

‎frontends/main/src/app/departments/page.tsx

+16-4
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,27 @@
11
import React from "react"
22
import { Metadata } from "next"
3-
3+
import DepartmentListingPage from "@/app-pages/DepartmentListingPage/DepartmentListingPage"
44
import { standardizeMetadata } from "@/common/metadata"
5+
import { Hydrate } from "@tanstack/react-query"
6+
import { learningResourcesKeyFactory } from "api/hooks/learningResources"
7+
import { channelsKeyFactory } from "api/hooks/channels"
8+
import { prefetch } from "api/ssr/prefetch"
9+
510
export const metadata: Metadata = standardizeMetadata({
611
title: "Departments",
712
})
813

9-
import DepartmentListingPage from "@/app-pages/DepartmentListingPage/DepartmentListingPage"
14+
const Page: React.FC = async () => {
15+
const dehydratedState = await prefetch([
16+
channelsKeyFactory.countsByType("department"),
17+
learningResourcesKeyFactory.schools(),
18+
])
1019

11-
const Page: React.FC = () => {
12-
return <DepartmentListingPage />
20+
return (
21+
<Hydrate state={dehydratedState}>
22+
<DepartmentListingPage />
23+
</Hydrate>
24+
)
1325
}
1426

1527
export default Page

0 commit comments

Comments
 (0)