Skip to content

Commit

Permalink
Dynamic load prefetch warnings and add retry tolerance
Browse files Browse the repository at this point in the history
  • Loading branch information
jonkafton committed Jan 15, 2025
1 parent da024df commit c90f4cc
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 9 deletions.
4 changes: 3 additions & 1 deletion frontends/api/src/ssr/usePrefetchWarnings.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { renderHook } from "@testing-library/react"
import { renderHook, waitFor } from "@testing-library/react"
import { useQuery } from "@tanstack/react-query"
import { usePrefetchWarnings } from "./usePrefetchWarnings"
import { setupReactQueryTest } from "../hooks/test-utils"
Expand Down Expand Up @@ -35,6 +35,7 @@ describe("SSR prefetch warnings", () => {
initialProps: { queryClient },
})

await waitFor(() => expect(console.info).toHaveBeenCalledTimes(1))
expect(console.info).toHaveBeenCalledWith(
"The following queries were requested in first render but not prefetched.",
"If these queries are user-specific, they cannot be prefetched - responses are cached on public CDN.",
Expand Down Expand Up @@ -97,6 +98,7 @@ describe("SSR prefetch warnings", () => {
initialProps: { queryClient },
})

await waitFor(() => expect(console.info).toHaveBeenCalledTimes(1))
expect(console.info).toHaveBeenCalledWith(
"The following queries were prefetched on the server but not accessed during initial render.",
"If these queries are no longer in use they should removed from prefetch:",
Expand Down
46 changes: 40 additions & 6 deletions frontends/api/src/ssr/usePrefetchWarnings.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { useEffect } from "react"
import { useEffect, useState } from "react"
import type { Query, QueryClient, QueryKey } from "@tanstack/react-query"
import { useIsFetching } from "@tanstack/react-query"
import { useMounted } from "./useMounted"

const logQueries = (...args: [...string[], Query[]]) => {
const queries = args.pop() as Query[]
Expand All @@ -17,7 +19,13 @@ const logQueries = (...args: [...string[], Query[]]) => {
)
}

const PREFETCH_EXEMPT_QUERIES = [["userMe"]]
const PREFETCH_EXEMPT_QUERIES = [
["userMe"],
["userLists", "membershipList", "membershipList"],
["learningPaths", "membershipList", "membershipList"],
]

const RETRIES = process.env.JEST_WORKER_ID ? 1 : 10

/**
* Call this as high as possible in render tree to detect query usage on
Expand All @@ -39,13 +47,28 @@ export const usePrefetchWarnings = ({
*/
exemptions?: QueryKey[]
}) => {
const mounted = useMounted()
const [count, setCount] = useState(0)
const fetchingCount = useIsFetching()
const [potentialWarnings, setPotentialWarnings] = useState(true)

useEffect(() => {
if ((potentialWarnings && count < RETRIES) || count === RETRIES - 1) {
setTimeout(() => setCount(count + 1), 250)
}
}, [count, potentialWarnings])

/**
* NOTE: React renders components top-down, but effects run bottom-up, so
* this effect will run after all child effects.
*/
useEffect(
() => {
if (process.env.NODE_ENV === "production") {
if (
process.env.NODE_ENV === "production" ||
!mounted ||
fetchingCount > 0
) {
return
}

Expand All @@ -63,7 +86,7 @@ export const usePrefetchWarnings = ({
!query.isDisabled(),
)

if (potentialPrefetches.length > 0) {
if (potentialPrefetches.length > 0 && count === RETRIES) {
logQueries(
"The following queries were requested in first render but not prefetched.",
"If these queries are user-specific, they cannot be prefetched - responses are cached on public CDN.",
Expand All @@ -80,17 +103,28 @@ export const usePrefetchWarnings = ({
!query.isDisabled(),
)

if (unusedPrefetches.length > 0) {
if (unusedPrefetches.length > 0 && count === RETRIES) {
logQueries(
"The following queries were prefetched on the server but not accessed during initial render.",
"If these queries are no longer in use they should removed from prefetch:",
unusedPrefetches,
)
}

setPotentialWarnings(
potentialPrefetches.length > 0 || unusedPrefetches.length > 0,
)
},
// We only want to run this on initial render.
// (Aside: queryClient should be a singleton anyway)
// eslint-disable-next-line react-hooks/exhaustive-deps
[],
[mounted, fetchingCount, count],
)
}

const PrefetchWarnings = ({ queryClient }: { queryClient: QueryClient }) => {
usePrefetchWarnings({ queryClient })
return null
}

export default PrefetchWarnings
10 changes: 8 additions & 2 deletions frontends/main/src/app/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,21 @@ import {
} from "ol-components"
import { Provider as NiceModalProvider } from "@ebay/nice-modal-react"
import ConfiguredPostHogProvider from "@/page-components/ConfiguredPostHogProvider/ConfiguredPostHogProvider"
import { usePrefetchWarnings } from "api/ssr/usePrefetchWarnings"
import { AppProgressBar as ProgressBar } from "next-nprogress-bar"
import type { NProgressOptions } from "next-nprogress-bar"
import dynamic from "next/dynamic"
import { usePrefetchWarnings } from "api/ssr/usePrefetchWarnings"

const PrefetchWarnings = dynamic(() => import("api/ssr/usePrefetchWarnings"), {
ssr: false,
})

const PROGRESS_BAR_OPTS: NProgressOptions = { showSpinner: false }

export default function Providers({ children }: { children: React.ReactNode }) {
const queryClient = getQueryClient()

usePrefetchWarnings({ queryClient })
usePrefetchWarnings()

return (
<>
Expand All @@ -30,6 +35,7 @@ export default function Providers({ children }: { children: React.ReactNode }) {
shallowRouting
/>
<QueryClientProvider client={queryClient}>
<PrefetchWarnings queryClient={queryClient} />
<ConfiguredPostHogProvider>
<NextJsAppRouterCacheProvider>
<ThemeProvider>
Expand Down

0 comments on commit c90f4cc

Please sign in to comment.