From 46fffc81c9520b94694507cad232367ccf0cd825 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 20 Sep 2024 13:42:44 -0400 Subject: [PATCH 1/3] use react-query directly --- frontends/api/package.json | 3 +-- frontends/api/src/ssr.ts | 10 ---------- frontends/main/package.json | 1 + frontends/main/src/app/getQueryClient.ts | 2 +- frontends/main/src/app/page.tsx | 2 +- frontends/main/src/app/providers.tsx | 2 +- .../ResourceCarousel/ResourceCarousel.tsx | 6 +++++- frontends/main/src/test-utils/index.tsx | 4 ++-- yarn.lock | 1 + 9 files changed, 13 insertions(+), 18 deletions(-) delete mode 100644 frontends/api/src/ssr.ts diff --git a/frontends/api/package.json b/frontends/api/package.json index 7abfd01313..ecaa7c61c2 100644 --- a/frontends/api/package.json +++ b/frontends/api/package.json @@ -11,8 +11,7 @@ "./hooks/*": "./src/hooks/*/index.ts", "./constants": "./src/common/constants.ts", "./test-utils/factories": "./src/test-utils/factories/index.ts", - "./test-utils": "./src/test-utils/index.ts", - "./ssr": "./src/ssr.ts" + "./test-utils": "./src/test-utils/index.ts" }, "peerDependencies": { "react": "18.3.1" diff --git a/frontends/api/src/ssr.ts b/frontends/api/src/ssr.ts deleted file mode 100644 index 4d43062624..0000000000 --- a/frontends/api/src/ssr.ts +++ /dev/null @@ -1,10 +0,0 @@ -export { - isServer, - Hydrate, - QueryClient, - QueryClientProvider, - dehydrate, - useQueries, -} from "@tanstack/react-query" - -export type { UseQueryResult, UseQueryOptions } from "@tanstack/react-query" diff --git a/frontends/main/package.json b/frontends/main/package.json index 9fd961f258..57910a8d45 100644 --- a/frontends/main/package.json +++ b/frontends/main/package.json @@ -14,6 +14,7 @@ "@emotion/cache": "^11.13.1", "@mitodl/course-search-utils": "mitodl/course-search-utils.git#jk/5244-support-for-nextjs", "@remixicon/react": "^4.2.0", + "@tanstack/react-query": "^4.36.1", "api": "workspace:*", "formik": "^2.4.6", "lodash": "^4.17.21", diff --git a/frontends/main/src/app/getQueryClient.ts b/frontends/main/src/app/getQueryClient.ts index e5116d82cd..fbd2dda22e 100644 --- a/frontends/main/src/app/getQueryClient.ts +++ b/frontends/main/src/app/getQueryClient.ts @@ -1,6 +1,6 @@ // Based on https://tanstack.com/query/v5/docs/framework/react/guides/advanced-ssr -import { QueryClient, isServer } from "api/ssr" +import { QueryClient, isServer } from "@tanstack/react-query" function makeQueryClient() { return new QueryClient({ diff --git a/frontends/main/src/app/page.tsx b/frontends/main/src/app/page.tsx index 58ac06a9d1..bf6f2e5705 100644 --- a/frontends/main/src/app/page.tsx +++ b/frontends/main/src/app/page.tsx @@ -1,6 +1,6 @@ import React from "react" import type { Metadata } from "next" -import { dehydrate, Hydrate } from "api/ssr" +import { dehydrate, Hydrate } from "@tanstack/react-query" import HomePage from "@/app-pages/HomePage/HomePage" import * as carousels from "@/app-pages/HomePage/carousels" import { learningResourcesKeyFactory } from "api/hooks/learningResources" diff --git a/frontends/main/src/app/providers.tsx b/frontends/main/src/app/providers.tsx index 3d7793dd1b..80b9632231 100644 --- a/frontends/main/src/app/providers.tsx +++ b/frontends/main/src/app/providers.tsx @@ -2,7 +2,7 @@ import React from "react" import { getQueryClient } from "./getQueryClient" -import { QueryClientProvider } from "api/ssr" +import { QueryClientProvider } from "@tanstack/react-query" import { ThemeProvider, NextJsAppRouterCacheProvider } from "ol-components" import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" diff --git a/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.tsx b/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.tsx index 4bd1305825..125b98ecaf 100644 --- a/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.tsx +++ b/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.tsx @@ -15,7 +15,11 @@ import { import type { TabConfig } from "./types" import { LearningResource, PaginatedLearningResourceList } from "api" import { ResourceCard } from "../ResourceCard/ResourceCard" -import { useQueries, UseQueryResult, UseQueryOptions } from "api/ssr" +import { + useQueries, + UseQueryResult, + UseQueryOptions, +} from "@tanstack/react-query" const StyledCarousel = styled(Carousel)({ /** diff --git a/frontends/main/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index 62237c9a89..7890d1ab8f 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -1,9 +1,9 @@ /* eslint-disable import/no-extraneous-dependencies */ import React from "react" -import { QueryClientProvider } from "api/ssr" +import { QueryClientProvider } from "@tanstack/react-query" import { ThemeProvider } from "ol-components" import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" -import type { QueryClient } from "api/ssr" +import type { QueryClient } from "@tanstack/react-query" import { makeQueryClient } from "@/app/getQueryClient" import { render } from "@testing-library/react" diff --git a/yarn.lock b/yarn.lock index fde6e4a907..50bd44652e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14252,6 +14252,7 @@ __metadata: "@faker-js/faker": "npm:^8.4.1" "@mitodl/course-search-utils": "mitodl/course-search-utils.git#jk/5244-support-for-nextjs" "@remixicon/react": "npm:^4.2.0" + "@tanstack/react-query": "npm:^4.36.1" "@testing-library/jest-dom": "npm:^6.4.8" "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.5.2" From 6190f260eef9e557f3438fc49fa313ad4d29f594 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 20 Sep 2024 13:51:40 -0400 Subject: [PATCH 2/3] better query caching --- .../main/src/app/getQueryClient.test.tsx | 41 +++++++++++++++++++ frontends/main/src/app/getQueryClient.ts | 28 ++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 frontends/main/src/app/getQueryClient.test.tsx diff --git a/frontends/main/src/app/getQueryClient.test.tsx b/frontends/main/src/app/getQueryClient.test.tsx new file mode 100644 index 0000000000..457acc593f --- /dev/null +++ b/frontends/main/src/app/getQueryClient.test.tsx @@ -0,0 +1,41 @@ +import React from "react" +import { renderHook, waitFor } from "@testing-library/react" +import { allowConsoleErrors } from "ol-test-utilities" +import { QueryClientProvider, useQuery } from "@tanstack/react-query" +import { makeQueryClient } from "./getQueryClient" + +const getWrapper = () => { + const queryClient = makeQueryClient() + const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( + {children} + ) + return wrapper +} + +test.each([ + { status: 408, retries: 3 }, + { status: 429, retries: 3 }, + { status: 502, retries: 3 }, + { status: 503, retries: 3 }, + { status: 504, retries: 3 }, +])( + "should retry $status failures $retries times", + async ({ status, retries }) => { + allowConsoleErrors() + const wrapper = getWrapper() + const queryFn = jest.fn().mockRejectedValue({ response: { status } }) + const { result } = renderHook( + () => + useQuery(["test"], { + queryFn, + retryDelay: 0, + }), + { wrapper }, + ) + + await waitFor(() => { + expect(result.current.isError).toBe(true) + }) + expect(queryFn).toHaveBeenCalledTimes(retries + 1) + }, +) diff --git a/frontends/main/src/app/getQueryClient.ts b/frontends/main/src/app/getQueryClient.ts index fbd2dda22e..03e1626b8f 100644 --- a/frontends/main/src/app/getQueryClient.ts +++ b/frontends/main/src/app/getQueryClient.ts @@ -2,10 +2,34 @@ import { QueryClient, isServer } from "@tanstack/react-query" -function makeQueryClient() { +type MaybeHasStatus = { + response?: { + status?: number + } +} + +const RETRY_STATUS_CODES = [408, 429, 502, 503, 504] +const MAX_RETRIES = 3 + +const makeQueryClient = (): QueryClient => { return new QueryClient({ defaultOptions: { - queries: {}, + queries: { + refetchOnWindowFocus: false, + staleTime: Infinity, + retry: (failureCount, error) => { + const status = (error as MaybeHasStatus)?.response?.status + /** + * React Query's default behavior is to retry all failed queries 3 + * times. Many things (e.g., 403, 404) are not worth retrying. Let's + * just retry some explicit whitelist of status codes. + */ + if (status !== undefined && RETRY_STATUS_CODES.includes(status)) { + return failureCount < MAX_RETRIES + } + return false + }, + }, }, }) } From df93778f0ea183ae9214b943cd60d60cb52c55d4 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 20 Sep 2024 15:48:13 -0400 Subject: [PATCH 3/3] remove featured-courses prefetches --- frontends/main/src/app/page.tsx | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/frontends/main/src/app/page.tsx b/frontends/main/src/app/page.tsx index bf6f2e5705..b918282d1d 100644 --- a/frontends/main/src/app/page.tsx +++ b/frontends/main/src/app/page.tsx @@ -1,11 +1,6 @@ import React from "react" import type { Metadata } from "next" -import { dehydrate, Hydrate } from "@tanstack/react-query" import HomePage from "@/app-pages/HomePage/HomePage" -import * as carousels from "@/app-pages/HomePage/carousels" -import { learningResourcesKeyFactory } from "api/hooks/learningResources" -import { FeaturedApiFeaturedListRequest } from "api/v1" -import { getQueryClient } from "./getQueryClient" import { getMetadataAsync } from "@/common/metadata" export async function generateMetadata({ @@ -22,25 +17,7 @@ export async function generateMetadata({ } const Page: React.FC = async () => { - const queryClient = getQueryClient() - - /* - * Prefetch the first tab (All) of the carousel - */ - await queryClient.prefetchQuery( - learningResourcesKeyFactory.featured( - carousels.FEATURED_RESOURCES_CAROUSEL[0].data - .params as FeaturedApiFeaturedListRequest, - ), - ) - - const dehydratedState = dehydrate(queryClient) - - return ( - - - - ) + return } export default Page