diff --git a/docs/decisions/0014-rootLoader-revalidation.md b/docs/decisions/0014-rootLoader-revalidation.md new file mode 100644 index 0000000000..139505c253 --- /dev/null +++ b/docs/decisions/0014-rootLoader-revalidation.md @@ -0,0 +1,54 @@ +# 0014. Triggering Revalidation on the `rootLoader` Route Loader on Sub-Route Navigation + +## Status + +Accepted (January 2025) + +## Context + +In this route-based application, the `rootLoader` is responsible for pre-fetching common queries needed across multiple routes. However, when navigating between sub-routes (e.g., from the Dashboard to the Search page), the `rootLoader` doesn’t re-execute. This becomes a problem when a component in the target route accesses query data with `useQuery` that hasn’t already been pre-fetched by a loader. + +By default, this issue is masked due to the query client configuration where queries have `suspense: true`. In these cases, if a query hasn’t been pre-fetched in a route loader, the `useQuery` hook causes the component to suspend, triggering the `AppSuspenseFallback` component while the query resolves asynchronously. This fallback behavior hides the underlying problem, making it harder to identify when query data is being accessed without pre-fetching. + +## Decision + +To ensure data consistency and explicitly address the issue of missing pre-fetched queries, two key changes will be implemented: + +### 1. Remove the Parent `Suspense` and `AppSuspenseFallback` Component + +The parent `Suspense` boundary and its fallback (`AppSuspenseFallback`) will be removed from the `App` component. By default, the query client configuration uses `suspense: true`, which masks the issue of missing pre-fetched query data by suspending components and displaying the fallback while queries resolve asynchronously. Removing this fallback ensures that missing query data triggers explicit errors instead of silently deferring to the fallback. This change improves the navigation experience and makes it easier to debug and fix data-loading issues during development. + +### 2. Implement `shouldRevalidate` on the `rootLoader` + +React Router by default only revalidates loaders after specific actions, such as form submissions or mutations. It does not automatically revalidate loaders during navigation between sibling routes under the same parent. To address this, the `shouldRevalidate` function will be implemented on the `rootLoader` route, with logic explicitly tailored to handle sub-route navigation. + +The `shouldRevalidate` function will determine whether the loader should re-execute by checking the following: + +* If the pathname has not changed between the current and next URLs, no revalidation will occur. +* If the pathname indicates navigation to a sub-route within the parent route (e.g., an enterprise-specific route or its sub-routes), the `rootLoader` will revalidate to ensure all required query data is pre-fetched and available. + +This logic ensures that revalidation is scoped specifically to relevant sub-route transitions, avoiding unnecessary data fetching while ensuring consistent query cache availability. + +### Benefits of this Approach + +* Queries required by UI components in the target route are always pre-fetched and cached before rendering. +* Developers can easily identify and address missing pre-fetches through explicit errors instead of relying on silent fallback behavior. +* Data consistency is maintained across sibling route transitions, particularly during the incremental migration to a Backend-for-Frontend (BFF) or API Gateway architecture. + +## Consequences + +### Positive Outcomes + +* **Reliable Query Data Availability:** Ensures that all required query data is pre-fetched and cached during sub-route transitions, preventing issues where `useQuery` tries to access missing or stale data. +* **Explicit Error Handling:** Removing the `AppSuspenseFallback` ensures that missing pre-fetched data triggers explicit errors instead of being masked by the fallback. This simplifies debugging and leads to better long-term reliability. +* **Scoped and Efficient Revalidation:** The `shouldRevalidate` function selectively targets relevant sub-route transitions. Combined with `ensureQueryData`, it prevents redundant API requests for data that remains fresh, minimizing performance impact. +* **Improved Developer Workflow:** By exposing missing pre-fetches, the approach facilitates early identification and resolution of query-related issues, reducing risks of data inconsistencies. + +### Negative Outcomes + +* **Initial Debugging Effort:** Removing the fallback may reveal existing cases where query data was not pre-fetched. This change could introduce additional debugging during development as missing pre-fetches surface as explicit errors. However, addressing these issues early ensures long-term consistency in data-loading strategies by enforcing a robust pattern of pre-fetching query data in route loaders. +* **Small Overhead for Non-Fresh Data:** Revalidating the `rootLoader` during sub-route navigation may result in occasional additional backend calls if data is not fresh. However, these calls are scoped and optimized, ensuring minimal impact on performance. + +## Alternatives Considered + +* Keeping the parent `Suspense` and its fallback component (`AppSuspenseFallback`) in the `App` component would allow the application to handle missing pre-fetches silently, suspending components and showing a secondary loading state during navigation. While this approach mitigates user-facing errors, it obscures data-loading inconsistencies and makes debugging more difficult. Additionally, it risks subtle issues in route navigation where query data dependencies are unclear. By adopting the chosen approach, explicit error handling ensures better enforcement of consistent and predictable data-loading practices. diff --git a/src/components/app/App.jsx b/src/components/app/App.jsx index 2ff141af85..fca99c60cd 100644 --- a/src/components/app/App.jsx +++ b/src/components/app/App.jsx @@ -1,5 +1,5 @@ import { - Suspense, lazy, useEffect, useState, + Suspense, lazy, useEffect, useMemo, useState, } from 'react'; import { RouterProvider } from 'react-router-dom'; import { AppProvider } from '@edx/frontend-platform/react'; @@ -15,7 +15,7 @@ import { defaultQueryClientRetryHandler, } from '../../utils/common'; -import { AppSuspenseFallback, RouterFallback, createAppRouter } from './routes'; +import { RouterFallback, createAppRouter } from './routes'; // eslint-disable-next-line import/no-unresolved const ReactQueryDevtoolsProduction = lazy(() => import('@tanstack/react-query-devtools/production').then((d) => ({ @@ -63,14 +63,16 @@ const queryClient = new QueryClient({ }, }); -const router = createAppRouter(queryClient); - const App = () => { const [showReactQueryDevtools, setShowReactQueryDevtools] = useState(false); useEffect(() => { window.toggleReactQueryDevtools = () => setShowReactQueryDevtools((prevState) => !prevState); }); + // Create the app router during render vs. at the top-level of the module to ensure + // the logging and auth modules are initialized before the router is created. + const router = useMemo(() => createAppRouter(queryClient), []); + return ( @@ -80,12 +82,10 @@ const App = () => { )} - }> - } - /> - + } + /> ); diff --git a/src/components/app/App.test.jsx b/src/components/app/App.test.jsx new file mode 100644 index 0000000000..d59b8f0cb3 --- /dev/null +++ b/src/components/app/App.test.jsx @@ -0,0 +1,93 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { createMemoryRouter } from 'react-router-dom'; +import { act } from 'react-dom/test-utils'; +import App from './App'; +import { createAppRouter } from './routes'; + +jest.mock('./routes', () => ({ + ...jest.requireActual('./routes'), + createAppRouter: jest.fn(), +})); + +jest.mock('@tanstack/react-query-devtools', () => ({ + ReactQueryDevtools: () =>
ReactQueryDevtools
, +})); + +jest.mock('@tanstack/react-query-devtools/production', () => ({ + ReactQueryDevtools: () =>
ReactQueryDevtoolsProduction
, +})); + +jest.mock('@edx/frontend-platform/react', () => ({ + AppProvider: ({ children }) =>
{children}
, +})); + +describe('App', () => { + let queryClient; + + beforeEach(() => { + queryClient = new QueryClient(); + const mockRouter = createMemoryRouter( + [{ path: '/', element:
Mock Route
}], + { initialEntries: ['/'] }, + ); + createAppRouter.mockReturnValue(mockRouter); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('renders App component without errors', () => { + render( + + + , + ); + + expect(screen.getByTestId('app-provider')).toBeInTheDocument(); + }); + + test('toggles ReactQueryDevtoolsProduction visibility', async () => { + render(); + + expect(screen.getByTestId('react-query-devtools')).toBeInTheDocument(); + + // Toggle visibility ON + act(() => { + window.toggleReactQueryDevtools(); + }); + await waitFor(() => { + expect(screen.getByTestId('react-query-devtools-production')).toBeInTheDocument(); + }); + + // Toggle visibility OFF + act(() => { + window.toggleReactQueryDevtools(); + }); + await waitFor(() => { + expect(screen.queryByTestId('react-query-devtools-production')).not.toBeInTheDocument(); + }); + }); + + test('uses the custom router created by createAppRouter', () => { + render(); + + expect(createAppRouter).toHaveBeenCalledWith( + expect.objectContaining({ + defaultOptions: { + queries: expect.objectContaining({ + cacheTime: 1000 * 60 * 30, + keepPreviousData: true, + retry: expect.any(Function), + staleTime: 1000 * 20, + suspense: true, + }), + }, + }), + ); + + expect(screen.getByTestId('mock-route')).toBeInTheDocument(); + }); +}); diff --git a/src/components/app/data/hooks/useNProgressLoader.js b/src/components/app/data/hooks/useNProgressLoader.js index efaa061be0..a3139462aa 100644 --- a/src/components/app/data/hooks/useNProgressLoader.js +++ b/src/components/app/data/hooks/useNProgressLoader.js @@ -10,7 +10,7 @@ import useNotices from './useNotices'; // for quick route transitions. export const NPROGRESS_DELAY_MS = 300; -function useNProgressLoader() { +function useNProgressLoader(queryOptions = {}) { const { authenticatedUser } = useContext(AppContext); const isAuthenticatedUserHydrated = !!authenticatedUser?.extendedProfile; const navigation = useNavigation(); @@ -18,7 +18,7 @@ function useNProgressLoader() { const { data: noticeRedirectUrl, isLoading: isLoadingNotices, - } = useNotices(); + } = useNotices(queryOptions.useNotices); const hasNoticeRedirectUrl = !isLoadingNotices && !!noticeRedirectUrl; const isAppDataHydrated = isAuthenticatedUserHydrated && !hasNoticeRedirectUrl; diff --git a/src/components/app/data/hooks/useNProgressLoader.test.jsx b/src/components/app/data/hooks/useNProgressLoader.test.jsx index db87eaf228..69b0d8fda5 100644 --- a/src/components/app/data/hooks/useNProgressLoader.test.jsx +++ b/src/components/app/data/hooks/useNProgressLoader.test.jsx @@ -122,7 +122,19 @@ describe('useNProgressLoader', () => { expect(result.current).toBe(true); }); - it('should call nprogress done with hydrated user and no notices', async () => { + it.each([ + { hasNoticesEnabled: true }, + { hasNoticesEnabled: false }, + ])('should call nprogress done with hydrated user and no notices (%s)', async ({ hasNoticesEnabled }) => { + const noticesQueryResult = { + data: undefined, + isLoading: true, + }; + if (hasNoticesEnabled) { + noticesQueryResult.data = null; + noticesQueryResult.isLoading = false; + } + useNotices.mockReturnValue(noticesQueryResult); const Wrapper = ({ children }) => ( {children} diff --git a/src/components/app/data/hooks/useNotices.js b/src/components/app/data/hooks/useNotices.js index 821e8bd84d..d61dd2984f 100644 --- a/src/components/app/data/hooks/useNotices.js +++ b/src/components/app/data/hooks/useNotices.js @@ -1,5 +1,6 @@ import { useEffect } from 'react'; import { useQuery } from '@tanstack/react-query'; +import { getConfig } from '@edx/frontend-platform/config'; import { queryNotices } from '../queries'; @@ -10,7 +11,8 @@ import { queryNotices } from '../queries'; function useNotices(queryOptions = {}) { const queryResults = useQuery({ ...queryNotices(), - queryOptions, + enabled: !!getConfig().ENABLE_NOTICES, + ...queryOptions, }); useEffect(() => { diff --git a/src/components/app/data/hooks/useNotices.test.jsx b/src/components/app/data/hooks/useNotices.test.jsx index c4f9ca01ce..b31bacfaa2 100644 --- a/src/components/app/data/hooks/useNotices.test.jsx +++ b/src/components/app/data/hooks/useNotices.test.jsx @@ -1,5 +1,6 @@ import { QueryClientProvider } from '@tanstack/react-query'; import { renderHook } from '@testing-library/react-hooks'; +import { getConfig } from '@edx/frontend-platform/config'; import { queryClient } from '../../../../utils/tests'; import useNotices from './useNotices'; @@ -10,6 +11,11 @@ jest.mock('../services', () => ({ fetchNotices: jest.fn().mockResolvedValue(null), })); +jest.mock('@edx/frontend-platform/config', () => ({ + ...jest.requireActual('@edx/frontend-platform/config'), + getConfig: jest.fn(), +})); + const wrapper = ({ children }) => ( {children} @@ -31,6 +37,21 @@ describe('useNotices', () => { beforeEach(() => { jest.clearAllMocks(); + getConfig.mockReturnValue({ ENABLE_NOTICES: true }); + }); + + it('should do nothing with notices disabled', () => { + getConfig.mockReturnValue({ ENABLE_NOTICES: false }); + + const { result } = renderHook(() => useNotices(), { wrapper }); + + expect(result.current).toEqual( + expect.objectContaining({ + data: undefined, + isLoading: true, + isFetching: false, + }), + ); }); it('should return null and NOT redirect with no notices', async () => { diff --git a/src/components/app/data/utils.js b/src/components/app/data/utils.js index 68ddb7f8a4..dac50c04d3 100644 --- a/src/components/app/data/utils.js +++ b/src/components/app/data/utils.js @@ -500,10 +500,29 @@ export function retrieveErrorMessage(error) { if (!error) { return null; } + let errorMessage = error.message; + // Identify development suspense error + const isDeveloperSuspenseError = ( + process.env.NODE_ENV === 'development' + && ['suspended while rendering', 'no fallback UI was specified'].every( + (str) => error.message.includes(str), + ) + ); + if (isDeveloperSuspenseError) { + errorMessage = ( + `A component or hook triggered suspense, possibly due to + missing pre-fetched data. Since \`suspense: true\` is configured + by default for all React Query queries, ensure that any necessary + API data accessed via \`useQuery\` is pre-fetched in a route loader + to avoid triggering suspense (loading states) unexpectedly. + Error: + ${errorMessage.split('\n')[0]}` + ); + } if (error.customAttributes) { - return error.customAttributes.httpErrorResponseData; + errorMessage += `\nCustom attributes: ${error.customAttributes.httpErrorResponseData}`; } - return error.message; + return errorMessage; } /** diff --git a/src/components/app/routes/AppSuspenseFallback.jsx b/src/components/app/routes/AppSuspenseFallback.jsx deleted file mode 100644 index 1dfee5114a..0000000000 --- a/src/components/app/routes/AppSuspenseFallback.jsx +++ /dev/null @@ -1,23 +0,0 @@ -import { useEffect } from 'react'; -import nprogress from 'accessible-nprogress'; - -import { NPROGRESS_DELAY_MS } from '../data/hooks/useNProgressLoader'; - -export function useNProgressLoaderWithoutRouter() { - useEffect(() => { - const timeoutId = setTimeout(() => { - nprogress.start(); - }, NPROGRESS_DELAY_MS); - return () => { - clearTimeout(timeoutId); - nprogress.done(); - }; - }, []); -} - -const AppSuspenseFallback = () => { - useNProgressLoaderWithoutRouter(); - return null; -}; - -export default AppSuspenseFallback; diff --git a/src/components/app/routes/RouteErrorBoundary.test.jsx b/src/components/app/routes/RouteErrorBoundary.test.jsx index f8614b7502..a1a8f5eb3f 100644 --- a/src/components/app/routes/RouteErrorBoundary.test.jsx +++ b/src/components/app/routes/RouteErrorBoundary.test.jsx @@ -18,9 +18,10 @@ const RouteErrorBoundaryWrapper = () => ( ); - +const originalNodeEnv = process.env.NODE_ENV; describe('RouteErrorBoundary', () => { beforeEach(() => { + process.env.NODE_ENV = originalNodeEnv; useRouteError.mockReturnValue(null); useAsyncError.mockReturnValue(null); @@ -43,6 +44,32 @@ describe('RouteErrorBoundary', () => { expect(screen.getByText('We apologize for the inconvenience. Please try again later.')).toBeInTheDocument(); }); + it('overrides default error message for development suspense errors', () => { + useRouteError.mockReturnValue(new Error('A React component suspended while rendering, but no fallback UI was specified')); + process.env.NODE_ENV = 'development'; + renderWithRouterProvider(); + expect(screen.getByText('An error occurred while processing your request')).toBeInTheDocument(); + expect(screen.getByText('We apologize for the inconvenience. Please try again later.')).toBeInTheDocument(); + expect(screen.getByText( + 'A component or hook triggered suspense, possibly due to missing pre-fetched data.', + { exact: false }, + )).toBeInTheDocument(); + }); + + it('uses customAttributes.httpErrorResponseData for axios errors', () => { + const error = new Error('RouteErrorWithCustomAttributes'); + error.customAttributes = { + httpErrorResponseData: { + status: 404, + }, + }; + useRouteError.mockReturnValue(error); + renderWithRouterProvider(); + expect(screen.getByText('An error occurred while processing your request')).toBeInTheDocument(); + expect(screen.getByText('We apologize for the inconvenience. Please try again later.')).toBeInTheDocument(); + expect(screen.getByText('Custom attributes:', { exact: false })).toBeInTheDocument(); + }); + it('displays the update available modal correctly when there is a ChunkLoadError route error', async () => { const chunkLoadError = new Error('ChunkLoadError'); chunkLoadError.name = 'ChunkLoadError'; diff --git a/src/components/app/routes/RouterFallback.jsx b/src/components/app/routes/RouterFallback.jsx index ee7c4cb50a..63b06aee70 100644 --- a/src/components/app/routes/RouterFallback.jsx +++ b/src/components/app/routes/RouterFallback.jsx @@ -1,7 +1,12 @@ import { useNProgressLoader } from '../data'; const RouterFallback = () => { - useNProgressLoader(); + const queryOptions = { + useNotices: { + suspense: false, + }, + }; + useNProgressLoader(queryOptions); return null; }; diff --git a/src/components/app/routes/createAppRouter.test.jsx b/src/components/app/routes/createAppRouter.test.jsx index 80f810af44..47602f7a14 100644 --- a/src/components/app/routes/createAppRouter.test.jsx +++ b/src/components/app/routes/createAppRouter.test.jsx @@ -1,5 +1,6 @@ import { - act, render, screen, waitFor, + act, + render, screen, waitFor, } from '@testing-library/react'; import { Outlet, RouterProvider } from 'react-router-dom'; import { IntlProvider } from '@edx/frontend-platform/i18n'; @@ -248,6 +249,10 @@ describe('createAppRouter', () => { expectedRouteTestId, expectedRouteLoaders, }) => { + // Update the current route path + window.history.pushState({}, '', currentRoutePath); + + // Render the app router const router = createAppRouter(mockQueryClient); render( @@ -259,15 +264,11 @@ describe('createAppRouter', () => { expect(makeRootLoader).toHaveBeenCalledTimes(1); expect(makeRootLoader).toHaveBeenCalledWith(mockQueryClient); expect(screen.getByTestId('root')).toBeInTheDocument(); - expect(screen.getByTestId('layout')).toBeInTheDocument(); - }); - - act(() => { - router.navigate(currentRoutePath); - }); - - await waitFor(() => { expect(screen.getByTestId(expectedRouteTestId)).toBeInTheDocument(); + if (expectedRouteTestId !== 'invite') { + // The invite routes are not associated with the `rootLoader` and `Layout` route + expect(screen.getByTestId('layout')).toBeInTheDocument(); + } }); if (expectedRouteLoaders.length > 0) { @@ -279,6 +280,37 @@ describe('createAppRouter', () => { }); } }); + + it('renders and revalidates rootLoader appropriately when navigating sub-routes (%s)', async () => { + // Create custom mocks + const rootLoaderFn = jest.fn().mockReturnValue(null); + makeRootLoader.mockReturnValue(rootLoaderFn); + + // Render the app router + const router = createAppRouter(mockQueryClient); + render( + + + , + ); + + // Assert initial load + await waitFor(() => { + expect(makeRootLoader).toHaveBeenCalledTimes(1); + expect(makeRootLoader).toHaveBeenCalledWith(mockQueryClient); + expect(rootLoaderFn).toHaveBeenCalledTimes(1); + }); + + // Trigger navigation to the next route + act(() => { + router.navigate('/test-enterprise/search'); + }); + + // Assert revalidation behavior + await waitFor(() => { + expect(rootLoaderFn).toHaveBeenCalledTimes(2); // Called again + }); + }); }); describe('getRouteLoader', () => { diff --git a/src/components/app/routes/createAppRouter.ts b/src/components/app/routes/createAppRouter.ts index b71c9d8b90..2845e07684 100644 --- a/src/components/app/routes/createAppRouter.ts +++ b/src/components/app/routes/createAppRouter.ts @@ -8,7 +8,7 @@ import { getRoutes } from '../../../routes'; * @param {Object} queryClient React Query query client. * @returns {Object} React Router browser router. */ -export default function createAppRouter(queryClient: Types.QueryClient) { +export default function createAppRouter(queryClient: Types.QueryClient): Types.Router { const { routes } = getRoutes(queryClient); const router = createBrowserRouter(routes); return router; diff --git a/src/components/app/routes/data/utils.js b/src/components/app/routes/data/utils.js index 5ec90b2887..20ea81b26e 100644 --- a/src/components/app/routes/data/utils.js +++ b/src/components/app/routes/data/utils.js @@ -1,22 +1,16 @@ import { generatePath, matchPath, redirect } from 'react-router-dom'; import { getConfig } from '@edx/frontend-platform'; import { - AxiosJwtAuthService, - configure as configureAuth, fetchAuthenticatedUser, getLoginRedirectUrl, } from '@edx/frontend-platform/auth'; -import { - configure as configureLogging, - getLoggingService, - NewRelicLoggingService, -} from '@edx/frontend-platform/logging'; import { getProxyLoginUrl } from '@edx/frontend-enterprise-logistration'; import Cookies from 'universal-cookie'; import { activateOrAutoApplySubscriptionLicense, addLicenseToSubscriptionLicensesByStatus, + queryAcademiesList, queryBrowseAndRequestConfiguration, queryContentHighlightsConfiguration, queryCouponCodeRequests, @@ -116,24 +110,30 @@ export async function ensureEnterpriseAppData({ ); } enterpriseAppDataQueries.push(...[ + // Redeemable Learner Credit Policies queryClient.ensureQueryData( queryRedeemablePolicies({ enterpriseUuid: enterpriseCustomer.uuid, lmsUserId: userId, }), ), + // Enterprise Coupon Codes queryClient.ensureQueryData( queryCouponCodes(enterpriseCustomer.uuid), ), + // Enterprise Learner Offers queryClient.ensureQueryData( queryEnterpriseLearnerOffers(enterpriseCustomer.uuid), ), + // Browse and Request Configuration queryClient.ensureQueryData( queryBrowseAndRequestConfiguration(enterpriseCustomer.uuid), ), + // License Requests queryClient.ensureQueryData( queryLicenseRequests(enterpriseCustomer.uuid, userEmail), ), + // Coupon Code Requests queryClient.ensureQueryData( queryCouponCodeRequests(enterpriseCustomer.uuid, userEmail), ), @@ -141,13 +141,20 @@ export async function ensureEnterpriseAppData({ queryClient.ensureQueryData( queryContentHighlightsConfiguration(enterpriseCustomer.uuid), ), + // Academies List + queryClient.ensureQueryData( + queryAcademiesList(enterpriseCustomer.uuid), + ), ]); if (getConfig().ENABLE_NOTICES) { enterpriseAppDataQueries.push( + // Notices queryClient.ensureQueryData(queryNotices()), ); } + + // Ensure all enterprise app data queries are resolved. await Promise.all(enterpriseAppDataQueries); } @@ -208,17 +215,6 @@ export function redirectToRemoveTrailingSlash(requestUrl) { throw redirect(requestUrl.pathname.slice(0, -1)); } -// Configure the logging and authentication services, only for non-test environments. -if (process.env.NODE_ENV !== 'test') { - configureLogging(NewRelicLoggingService, { - config: getConfig(), - }); - configureAuth(AxiosJwtAuthService, { - loggingService: getLoggingService(), - config: getConfig(), - }); -} - /** * Ensures that the user is authenticated. If not, redirects to the login page. * @param {URL} requestUrl - The current request URL to redirect back to if the diff --git a/src/components/app/routes/index.js b/src/components/app/routes/index.js index 052f479540..cf25703dcb 100644 --- a/src/components/app/routes/index.js +++ b/src/components/app/routes/index.js @@ -1,5 +1,4 @@ export { default as RouterFallback } from './RouterFallback'; -export { default as AppSuspenseFallback } from './AppSuspenseFallback'; export { default as RouteErrorBoundary } from './RouteErrorBoundary'; export { default as createAppRouter } from './createAppRouter'; diff --git a/src/components/app/routes/loaders/tests/rootLoader.test.jsx b/src/components/app/routes/loaders/tests/rootLoader.test.jsx index 444e9ca2e8..4671112586 100644 --- a/src/components/app/routes/loaders/tests/rootLoader.test.jsx +++ b/src/components/app/routes/loaders/tests/rootLoader.test.jsx @@ -9,6 +9,7 @@ import { activateOrAutoApplySubscriptionLicense, extractEnterpriseCustomer, getBaseSubscriptionsData, + queryAcademiesList, queryBrowseAndRequestConfiguration, queryContentHighlightsConfiguration, queryCouponCodeRequests, @@ -299,13 +300,13 @@ describe('rootLoader', () => { await waitFor(() => { // Assert that the expected number of queries were made. - let expectedQueryCount = 9; + let expectedQueryCount = 10; if (enterpriseSlug !== activeEnterpriseCustomer.slug) { if (!(isLinked || isStaffUser)) { expectedQueryCount = 2; } } else if (hasResolvedBFFQuery) { - expectedQueryCount = 8; + expectedQueryCount = 9; } expect(mockQueryClient.ensureQueryData).toHaveBeenCalledTimes(expectedQueryCount); }); @@ -430,5 +431,14 @@ describe('rootLoader', () => { queryFn: expect.any(Function), }), ); + + // Academies list query + const academiesListQuery = queryAcademiesList(enterpriseCustomer.uuid); + expect(mockQueryClient.ensureQueryData).toHaveBeenCalledWith( + expect.objectContaining({ + queryKey: academiesListQuery.queryKey, + queryFn: expect.any(Function), + }), + ); }); }); diff --git a/src/components/search/SearchPage.jsx b/src/components/search/SearchPage.jsx index 1b88a680e3..8121f1e642 100644 --- a/src/components/search/SearchPage.jsx +++ b/src/components/search/SearchPage.jsx @@ -8,9 +8,8 @@ import { features } from '../../config'; import { useHasValidLicenseOrSubscriptionRequestsEnabled } from '../app/data'; const SearchPage = () => { - const hasValidLicenseOrSubRequest = useHasValidLicenseOrSubscriptionRequestsEnabled(); - const intl = useIntl(); + const hasValidLicenseOrSubRequest = useHasValidLicenseOrSubscriptionRequestsEnabled(); const enableVideos = ( features.FEATURE_ENABLE_VIDEO_CATALOG && hasValidLicenseOrSubRequest diff --git a/src/routes.tsx b/src/routes.tsx index f9314201f9..5a5877e602 100644 --- a/src/routes.tsx +++ b/src/routes.tsx @@ -191,6 +191,15 @@ function getEnterpriseSlugRoutes(queryClient?: Types.QueryClient) { loader: getRouteLoader(makeRootLoader, queryClient), element: , children: enterpriseSlugChildRoutes, + shouldRevalidate: ({ currentUrl, nextUrl, defaultShouldRevalidate }) => { + // If the pathname changed, we should revalidate + if (currentUrl.pathname !== nextUrl.pathname) { + return true; + } + + // If the pathname didn't change, fallback to the default behavior + return defaultShouldRevalidate; + }, }, ]; return enterpriseSlugRoutes; diff --git a/src/types.d.ts b/src/types.d.ts index 9c0c4f9e71..aa086e7516 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -20,6 +20,7 @@ export type RouteLoaderFunctionArgs = import('react-router-dom').LoaderFunctionA export type MakeRouteLoaderFunction = (queryClient?: QueryClient) => RouteLoaderFunction; export type MakeRouteLoaderFunctionWithQueryClient = (queryClient: QueryClient) => RouteLoaderFunction; export type RouteObject = import('react-router-dom').RouteObject; +export type Router = import('@remix-run/router').Router; // Application Data (general) export interface AuthenticatedUser {