Refactor to use a single useQueries hook between renders

Author: jonkafton

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

@@ -427,6 +427,13 @@ const useSchoolsList = () => {
   return useQuery(learningResources.schools())
 }
 
+/*
+ * Not intended to be imported except for special cases.
+ * It's used in the ResourceCarousel to dynamically build a single useQueries hook
+ * from config because a React component cannot conditionally call hooks during renders.
+ */
+export { default as learningResourcesKeyFactory } from "./keyFactory"
+
 export {
   useLearningResourcesList,
   useFeaturedLearningResourcesList,

frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx

@@ -1,9 +1,9 @@
 import React from "react"
 import {
   useFeaturedLearningResourcesList,
-  useLearningResourcesList,
-  useLearningResourcesSearch,
+  learningResourcesKeyFactory,
 } from "api/hooks/learningResources"
+
 import {
   Carousel,
   TabButton,
@@ -13,14 +13,10 @@ import {
   styled,
   Typography,
 } from "ol-components"
-import type {
-  TabConfig,
-  ResourceDataSource,
-  SearchDataSource,
-  FeaturedDataSource,
-} from "./types"
+import type { TabConfig, FeaturedDataSource } from "./types"
 import { LearningResource } from "api"
 import { ResourceCard } from "../ResourceCard/ResourceCard"
+import { useQueries } from "@tanstack/react-query"
 
 const StyledCarousel = styled(Carousel)({
   /**
@@ -40,92 +36,13 @@ const StyledCarousel = styled(Carousel)({
   },
 })
 
-type DataPanelProps<T extends TabConfig["data"] = TabConfig["data"]> = {
-  dataConfig: T
-  isLoading?: boolean
-  children: ({
-    resources,
-    childrenLoading,
-  }: {
-    resources: LearningResource[]
-    childrenLoading: boolean
-  }) => React.ReactNode
-}
-
 type LoadTabButtonProps = {
   config: FeaturedDataSource
   label: React.ReactNode
   key: number
   value: string
 }
 
-const ResourcesData: React.FC<DataPanelProps<ResourceDataSource>> = ({
-  dataConfig,
-  children,
-}) => {
-  const { data, isLoading } = useLearningResourcesList(dataConfig.params)
-  return children({
-    resources: data?.results ?? [],
-    childrenLoading: isLoading,
-  })
-}
-
-const SearchData: React.FC<DataPanelProps<SearchDataSource>> = ({
-  dataConfig,
-  children,
-}) => {
-  const { data, isLoading } = useLearningResourcesSearch(dataConfig.params)
-  return children({
-    resources: data?.results ?? [],
-    childrenLoading: isLoading,
-  })
-}
-
-const FeaturedData: React.FC<DataPanelProps<FeaturedDataSource>> = ({
-  dataConfig,
-  children,
-}) => {
-  const { data, isLoading } = useFeaturedLearningResourcesList(
-    dataConfig.params,
-  )
-  return children({
-    resources: data?.results ?? [],
-    childrenLoading: isLoading,
-  })
-}
-
-/**
- * A wrapper to load data based `TabConfig.data`.
- *
- * For each `TabConfig.data.type`, a different API endpoint, and hence
- * react-query hook, is used. Since hooks can't be called conditionally within
- * a single component, each type of data is handled in a separate component.
- */
-const DataPanel: React.FC<DataPanelProps> = ({
-  dataConfig,
-  isLoading,
-  children,
-}) => {
-  if (!isLoading) {
-    switch (dataConfig.type) {
-      case "resources":
-        return <ResourcesData dataConfig={dataConfig}>{children}</ResourcesData>
-      case "lr_search":
-        return <SearchData dataConfig={dataConfig}>{children}</SearchData>
-      case "lr_featured":
-        return <FeaturedData dataConfig={dataConfig}>{children}</FeaturedData>
-      default:
-        // @ts-expect-error This will always be an error if the switch statement
-        // is exhaustive since dataConfig will have type `never`
-        throw new Error(`Unknown data type: ${dataConfig.type}`)
-    }
-  } else
-    return children({
-      resources: [],
-      childrenLoading: true,
-    })
-}
-
 /**
  * Tab button that loads the resource, so we can determine if it needs to be
  * displayed or not. This shouldn't cause double-loading since React Query
@@ -195,48 +112,45 @@ const TabsList = styled(TabButtonList)({
 
 type ContentProps = {
   resources: LearningResource[]
-  isLoading?: boolean
+  childrenLoading?: boolean
   tabConfig: TabConfig
 }
 
 type PanelChildrenProps = {
   config: TabConfig[]
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  queries: any
   children: (props: ContentProps) => React.ReactNode
-  isLoading?: boolean
 }
 const PanelChildren: React.FC<PanelChildrenProps> = ({
   config,
+  queries,
   children,
-  isLoading,
 }) => {
   if (config.length === 1) {
-    return (
-      <DataPanel dataConfig={config[0].data} isLoading={isLoading}>
-        {({ resources, childrenLoading }) =>
-          children({
-            resources,
-            isLoading: childrenLoading || isLoading,
-            tabConfig: config[0],
-          })
-        }
-      </DataPanel>
-    )
+    const { data, isLoading } = queries[0]
+    const resources = data?.results ?? []
+    return children({
+      resources,
+      childrenLoading: isLoading,
+      tabConfig: config[0],
+    })
   }
   return (
     <>
-      {config.map((tabConfig, index) => (
-        <StyledTabPanel key={index} value={index.toString()}>
-          <DataPanel dataConfig={tabConfig.data} isLoading={isLoading}>
-            {({ resources, childrenLoading }) =>
-              children({
-                resources,
-                isLoading: childrenLoading || isLoading,
-                tabConfig,
-              })
-            }
-          </DataPanel>
-        </StyledTabPanel>
-      ))}
+      {config.map((tabConfig, index) => {
+        const { data, isLoading } = queries[index]
+        const resources = data?.results ?? []
+        return (
+          <StyledTabPanel key={index} value={index.toString()}>
+            {children({
+              resources,
+              childrenLoading: isLoading,
+              tabConfig,
+            })}
+          </StyledTabPanel>
+        )
+      })}
     </>
   )
 }
@@ -258,6 +172,7 @@ type ResourceCarouselProps = {
   title: string
   className?: string
   isLoading?: boolean
+  "data-testid"?: string
 }
 /**
  * A tabbed carousel that fetches resources based on the configuration provided.
@@ -275,12 +190,27 @@ const ResourceCarousel: React.FC<ResourceCarouselProps> = ({
   title,
   className,
   isLoading,
+  "data-testid": dataTestId,
 }) => {
   const [tab, setTab] = React.useState("0")
   const [ref, setRef] = React.useState<HTMLDivElement | null>(null)
 
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const queries = useQueries<any>({
+    queries: config.map((tab) => {
+      switch (tab.data.type) {
+        case "resources":
+          return learningResourcesKeyFactory.list(tab.data.params)
+        case "lr_search":
+          return learningResourcesKeyFactory.search(tab.data.params)
+        case "lr_featured":
+          return learningResourcesKeyFactory.featured(tab.data.params)
+      }
+    }),
+  })
+
   return (
-    <MobileOverflow className={className}>
+    <MobileOverflow className={className} data-testid={dataTestId}>
       <TabContext value={tab}>
         <HeaderRow>
           <HeaderText variant="h4">{title}</HeaderText>
@@ -309,8 +239,8 @@ const ResourceCarousel: React.FC<ResourceCarouselProps> = ({
             </ControlsContainer>
           ) : null}
         </HeaderRow>
-        <PanelChildren config={config} isLoading={isLoading}>
-          {({ resources, isLoading: childrenLoading, tabConfig }) => (
+        <PanelChildren config={config} queries={queries}>
+          {({ resources, childrenLoading, tabConfig }) => (
             <StyledCarousel arrowsContainer={ref}>
               {isLoading || childrenLoading
                 ? Array.from({ length: 6 }).map((_, index) => (

frontends/mit-open/src/pages/DashboardPage/DashboardPage.tsx

@@ -259,6 +259,13 @@ const CarouselContainer = styled.div(({ theme }) => ({
   },
 }))
 
+const StyledResourceCarousel = styled(ResourceCarousel)(({ theme }) => ({
+  padding: "40px 0",
+  [theme.breakpoints.down("md")]: {
+    padding: "16px 0",
+  },
+}))
+
 interface UserMenuTabProps {
   icon: React.ReactNode
   text: string
@@ -487,16 +494,16 @@ const DashboardPage: React.FC = () => {
                         title={"Courses with Certificates"}
                         isLoading={isLoadingProfile}
                         config={CERTIFICATE_COURSES_CAROUSEL}
+                        data-testid="certification-carousel"
                       />
                     </CarouselContainer>
                   ) : (
-                    <CarouselContainer data-testid="free-carousel">
-                      <ResourceCarousel
-                        title={"Free courses"}
-                        isLoading={isLoadingProfile}
-                        config={FREE_COURSES_CAROUSEL}
-                      />
-                    </CarouselContainer>
+                    <StyledResourceCarousel
+                      title="Free courses"
+                      isLoading={isLoadingProfile}
+                      config={FREE_COURSES_CAROUSEL}
+                      data-testid="free-carousel"
+                    />
                   )}
                   <CarouselContainer data-testid="new-learning-resources-carousel">
                     <ResourceCarousel