Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: upgrade react router to v6 #776

Merged
merged 8 commits into from
Oct 10, 2023
4,195 changes: 2,377 additions & 1,818 deletions package-lock.json

Large diffs are not rendered by default.

17 changes: 8 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,19 @@
],
"dependencies": {
"@edx/brand": "npm:@edx/[email protected]",
"@edx/frontend-component-footer": "12.1.2",
"@edx/frontend-enterprise-catalog-search": "4.7.1",
"@edx/frontend-enterprise-hotjar": "1.4.0",
"@edx/frontend-enterprise-logistration": "3.4.0",
"@edx/frontend-enterprise-utils": "3.4.0",
"@edx/frontend-platform": "4.6.3",
"@edx/frontend-component-footer": "12.2.0",
"@edx/frontend-enterprise-catalog-search": "5.0.0",
"@edx/frontend-enterprise-hotjar": "2.0.0",
"@edx/frontend-enterprise-logistration": "4.0.0",
"@edx/frontend-enterprise-utils": "4.0.0",
"@edx/frontend-platform": "5.0.0",
"@edx/paragon": "20.45.4",
"@tanstack/react-query": "4.28.0",
"@tanstack/react-query-devtools": "4.29.0",
"algoliasearch": "4.6.0",
"axios": "0.21.1",
"classnames": "2.2.6",
"color": "3.1.3",
"connected-react-router": "6.9.2",
"core-js": "3.7.0",
"dayjs": "^1.11.9",
"dompurify": "2.3.6",
Expand All @@ -43,8 +42,8 @@
"react-instantsearch-dom": "6.38.1",
"react-parallax": "3.3.0",
"react-redux": "7.2.2",
"react-router": "4.3.1",
"react-router-dom": "5.2.0",
"react-router": "6.16.0",
"react-router-dom": "6.16.0",
"react-router-hash-link": "2.3.1",
"react-scroll": "1.8.4",
"react-string-replace": "1.1.0",
Expand Down
61 changes: 37 additions & 24 deletions src/components/app/App.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { useEffect } from 'react';
import { Switch, Route, Redirect } from 'react-router-dom';
import { AppProvider, AuthenticatedPageRoute, PageRoute } from '@edx/frontend-platform/react';
import {
Routes, Route, Navigate, useLocation,
} from 'react-router-dom';
import { AppProvider, AuthenticatedPageRoute, PageWrap } from '@edx/frontend-platform/react';
import { logError } from '@edx/frontend-platform/logging';
import { initializeHotjar } from '@edx/frontend-enterprise-hotjar';
import {
Expand All @@ -25,6 +27,15 @@
// Create a query client for @tanstack/react-query
const queryClient = new QueryClient();

const TruncatedLocation = () => {
const location = useLocation();

Check warning on line 31 in src/components/app/App.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/app/App.jsx#L30-L31

Added lines #L30 - L31 were not covered by tests

if (location.pathname.endsWith('/')) {
return <Navigate to={location.pathname.slice(0, -1)} replace />;

Check warning on line 34 in src/components/app/App.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/app/App.jsx#L34

Added line #L34 was not covered by tests
}
return null;

Check warning on line 36 in src/components/app/App.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/app/App.jsx#L36

Added line #L36 was not covered by tests
};

const App = () => {
// hotjar initialization
useEffect(() => {
Expand All @@ -48,34 +59,36 @@
<NoticesProvider>
<ToastsProvider>
<Toasts />
<Switch>
{/* always remove trailing slashes from any route */}
<Redirect from="/:url*(/+)" to={global.location.pathname.slice(0, -1)} />
{/* page routes for the app */}
<AuthenticatedPageRoute exact path="/" component={EnterpriseCustomerRedirect} />
<AuthenticatedPageRoute exact path="/r/:redirectPath+" component={EnterprisePageRedirect} />
<PageRoute exact path="/invite/:enterpriseCustomerInviteKey" component={EnterpriseInvitePage} />
<PageRoute
exact
{/* always remove trailing slashes from any route */}
<TruncatedLocation />
{/* page routes for the app */}
<Routes>
<Route path="/" element={<AuthenticatedPageRoute><EnterpriseCustomerRedirect /></AuthenticatedPageRoute>} />
<Route path="/r/*" element={<AuthenticatedPageRoute><EnterprisePageRedirect /></AuthenticatedPageRoute>} />
<Route path="/invite/:enterpriseCustomerInviteKey" element={<PageWrap><EnterpriseInvitePage /></PageWrap>} />
<Route
path="/:enterpriseSlug/executive-education-2u"
render={(routeProps) => (
<AuthenticatedPage>
<ExecutiveEducation2UPage {...routeProps} />
</AuthenticatedPage>
element={(
<PageWrap>
<AuthenticatedPage>
<ExecutiveEducation2UPage />
</AuthenticatedPage>
</PageWrap>
)}
/>
<PageRoute
exact
<Route
path="/:enterpriseSlug/executive-education-2u/enrollment-completed"
render={(routeProps) => (
<AuthenticatedPage>
<EnrollmentCompleted {...routeProps} />
</AuthenticatedPage>
element={(
<PageWrap>
<AuthenticatedPage>
<EnrollmentCompleted />
</AuthenticatedPage>
</PageWrap>
)}
/>
<Route path="/:enterpriseSlug" component={EnterpriseAppPageRoutes} />
<PageRoute path="*" component={NotFoundPage} />
</Switch>
<Route path="/:enterpriseSlug/*" element={<EnterpriseAppPageRoutes />} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: why do we need to add the * at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: why do we need to add the * at the end?

React Router V6 handles nested routes a little bit differently. So in order to match the descendent routes we have to add this *. [ref]

<Route path="*" element={<PageWrap><NotFoundPage /></PageWrap>} />
</Routes>
</ToastsProvider>
</NoticesProvider>
</AppProvider>
Expand Down
46 changes: 24 additions & 22 deletions src/components/app/EnterpriseAppPageRoutes.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { PageRoute } from '@edx/frontend-platform/react';
import { PageWrap } from '@edx/frontend-platform/react';
import { Route, Routes } from 'react-router-dom';

import { DashboardPage } from '../dashboard';
import { CoursePage } from '../course';
Expand All @@ -16,27 +17,28 @@
// to reduce API calls by 2 (DashboardPage, CoursePage, SearchPage) or by 3 ( + AuthenticatedPage) if created in App.jsx
const EnterpriseAppPageRoutes = () => (
<AuthenticatedUserSubsidyPage>
<PageRoute exact path="/:enterpriseSlug" component={DashboardPage} />
<PageRoute
exact
path={['/:enterpriseSlug/search', '/:enterpriseSlug/search/:pathwayUUID']}
component={SearchPage}
/>
<PageRoute exact path="/:enterpriseSlug/course/:courseKey" component={CoursePage} />
<PageRoute path="/:enterpriseSlug/:courseType/course/:courseKey" component={CoursePage} />
{features.ENABLE_PROGRAMS && (
<PageRoute exact path="/:enterpriseSlug/program/:programUuid" component={ProgramPage} />
)}
{
// Deprecated URL, will be removed in the future.
<PageRoute exact path="/:enterpriseSlug/program-progress/:programUUID" component={ProgramProgressRedirect} />
}
<PageRoute exact path="/:enterpriseSlug/program/:programUUID/progress" component={ProgramProgressPage} />
<PageRoute exact path="/:enterpriseSlug/skills-quiz" component={SkillsQuizPage} />
<PageRoute exact path="/:enterpriseSlug/licenses/:activationKey/activate" component={LicenseActivationPage} />
{features.FEATURE_ENABLE_PATHWAY_PROGRESS && (
<PageRoute exact path="/:enterpriseSlug/pathway/:pathwayUUID/progress" component={PathwayProgressPage} />
)}
<Routes>
<Route path="/" element={<PageWrap><DashboardPage /></PageWrap>} />
{['search', 'search/:pathwayUUID'].map(route => (
<Route

Check warning on line 23 in src/components/app/EnterpriseAppPageRoutes.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/app/EnterpriseAppPageRoutes.jsx#L23

Added line #L23 was not covered by tests
Syed-Ali-Abbas-Zaidi marked this conversation as resolved.
Show resolved Hide resolved
key={route}
path={route}
element={<PageWrap><SearchPage /></PageWrap>}
/>
))}
<Route path="course/:courseKey/*" element={<PageWrap><CoursePage /></PageWrap>} />
<Route path=":courseType/course/:courseKey/*" element={<PageWrap><CoursePage /></PageWrap>} />
{features.ENABLE_PROGRAMS && (
<Route path="program/:programUuid" element={<PageWrap><ProgramPage /></PageWrap>} />

Check warning on line 32 in src/components/app/EnterpriseAppPageRoutes.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/app/EnterpriseAppPageRoutes.jsx#L32

Added line #L32 was not covered by tests
)}
<Route path="program-progress/:programUUID" element={<PageWrap><ProgramProgressRedirect /></PageWrap>} />
<Route path="program/:programUUID/progress" element={<PageWrap><ProgramProgressPage /></PageWrap>} />
<Route path="skills-quiz" element={<PageWrap><SkillsQuizPage /></PageWrap>} />
<Route path="licenses/:activationKey/activate" element={<PageWrap><LicenseActivationPage /></PageWrap>} />
{features.FEATURE_ENABLE_PATHWAY_PROGRESS && (
<Route exact path="pathway/:pathwayUUID/progress" element={<PageWrap><PathwayProgressPage /></PageWrap>} />

Check warning on line 39 in src/components/app/EnterpriseAppPageRoutes.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/app/EnterpriseAppPageRoutes.jsx#L39

Added line #L39 was not covered by tests
)}
</Routes>
</AuthenticatedUserSubsidyPage>
);

Expand Down
8 changes: 4 additions & 4 deletions src/components/course/CoursePage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, {
useCallback, useContext, useEffect, useMemo, useState,
} from 'react';
import {
useLocation, useParams, useHistory,
useLocation, useParams, useNavigate,
} from 'react-router-dom';
import { Helmet } from 'react-helmet';
import { Container } from '@edx/paragon';
Expand Down Expand Up @@ -63,7 +63,7 @@ const CoursePage = () => {
},
} = useEnterpriseCuration(enterpriseUUID);
const { pathname, search } = useLocation();
const history = useHistory();
const navigate = useNavigate();

const courseRunKey = useMemo(
() => {
Expand Down Expand Up @@ -226,9 +226,9 @@ const CoursePage = () => {
courseState.course,
enterpriseSlug,
);
history.replace(newUrl);
navigate(newUrl, { replace: true });
}
}, [enterpriseSlug, history, courseState, pathname]);
}, [enterpriseSlug, navigate, courseState, pathname]);

const subsidyRequestCatalogsApplicableToCourse = useMemo(() => {
const catalogsContainingCourse = new Set(courseState?.catalog?.catalogList);
Expand Down
6 changes: 3 additions & 3 deletions src/components/course/CourseRecommendationCard.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useContext, useMemo } from 'react';
import PropTypes from 'prop-types';
import cardFallbackImg from '@edx/brand/paragon/images/card-imagecap-fallback.png';
import { useHistory } from 'react-router-dom';
import { useNavigate } from 'react-router-dom';
import { AppContext } from '@edx/frontend-platform/react';
import { Card, Truncate } from '@edx/paragon';
import { sendEnterpriseTrackEvent } from '@edx/frontend-enterprise-utils';
Expand All @@ -15,7 +15,7 @@ export const SAME_PART_EVENT_NAME = 'edx.ui.enterprise.learner_portal.same.partn
const CourseRecommendationCard = ({ course, isPartnerRecommendation }) => {
const { enterpriseConfig: { slug, uuid } } = useContext(AppContext);
const eventName = isPartnerRecommendation ? SAME_PART_EVENT_NAME : COURSE_REC_EVENT_NAME;
const history = useHistory();
const navigate = useNavigate();
const cachedLinkToCourse = useMemo(
() => linkToCourse(course, slug),
[course, slug],
Expand Down Expand Up @@ -49,7 +49,7 @@ const CourseRecommendationCard = ({ course, isPartnerRecommendation }) => {
courseKey: course.key,
},
);
history.push(cachedLinkToCourse);
navigate(cachedLinkToCourse);
}}
>
<Card.ImageCap
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const wrapper = ({ children }) => (

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useRouteMatch: jest.fn().mockReturnValue({ url: '/test-enterprise/course/edX+DemoX' }),
useLocation: jest.fn().mockReturnValue({ pathname: '/test-enterprise/course/edX+DemoX' }),
}));

jest.mock('../useCourseRunCardHeading', () => jest.fn(() => 'Course started'));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useRouteMatch } from 'react-router-dom';
import { useLocation } from 'react-router-dom';

import { COURSE_AVAILABILITY_MAP } from '../../../data/constants';
import useCourseRunCardHeading from './useCourseRunCardHeading';
Expand Down Expand Up @@ -26,7 +26,7 @@ const useCourseRunCardData = ({
subsidyAccessPolicy,
userCanRequestSubsidyForCourse,
}) => {
const routeMatch = useRouteMatch();
const { pathname } = useLocation();
const {
key: contentKey,
availability,
Expand All @@ -35,7 +35,7 @@ const useCourseRunCardData = ({
const isCourseRunCurrent = availability === COURSE_AVAILABILITY_MAP.CURRENT;
const isUserEnrolled = !!userEnrollment;
const externalCourseEnrollmentUrl = getExternalCourseEnrollmentUrl({
currentRouteUrl: routeMatch.url,
currentRouteUrl: pathname,
});

// Get and return course run card data for display
Expand Down
17 changes: 9 additions & 8 deletions src/components/course/data/hooks.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
useCallback, useContext, useEffect, useMemo, useState,
} from 'react';
import { useHistory, useLocation, useRouteMatch } from 'react-router-dom';
import { useNavigate, useLocation } from 'react-router-dom';
import PropTypes from 'prop-types';
import { sendEnterpriseTrackEvent } from '@edx/frontend-enterprise-utils';
import { logError } from '@edx/frontend-platform/logging';
Expand Down Expand Up @@ -285,7 +285,7 @@
userSubsidyApplicableToCourse,
isExecutiveEducation2UCourse,
}) => {
const routeMatch = useRouteMatch();
const { pathname } = useLocation();
const config = getConfig();

const baseQueryParams = useMemo(() => {
Expand Down Expand Up @@ -331,7 +331,7 @@

if (isExecutiveEducation2UCourse) {
const externalCourseEnrollmentUrl = getExternalCourseEnrollmentUrl({
currentRouteUrl: routeMatch.url,
currentRouteUrl: pathname,
});
return externalCourseEnrollmentUrl;
}
Expand All @@ -352,7 +352,7 @@
courseRunKey,
enterpriseConfig.uuid,
isExecutiveEducation2UCourse,
routeMatch.url,
pathname,
location,
],
);
Expand All @@ -367,8 +367,8 @@
* @returns An object containing the Algolia objectId and queryId that led to a page view of the Course page.
*/
export const useExtractAndRemoveSearchParamsFromURL = () => {
const { search } = useLocation();
const history = useHistory();
const { pathname, search } = useLocation();
const navigate = useNavigate();
const [algoliaSearchParams, setAlgoliaSearchParams] = useState({});

const queryParams = useMemo(
Expand All @@ -385,12 +385,13 @@
});
queryParams.delete('queryId');
queryParams.delete('objectId');
history.replace({
navigate(pathname, {
search: queryParams.toString(),
replace: true,
});
}
},
[history, queryParams],
[navigate, queryParams, pathname],
);

return algoliaSearchParams;
Expand Down Expand Up @@ -672,12 +673,12 @@
return camelCaseObject(fetchLicenseSubsidyResponse.data);
}
} catch (error) {
logError(error);

Check warning on line 676 in src/components/course/data/hooks.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/course/data/hooks.jsx#L676

Added line #L676 was not covered by tests
if (onSubscriptionLicenseForCourseValidationError) {
onSubscriptionLicenseForCourseValidationError(error);

Check warning on line 678 in src/components/course/data/hooks.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/course/data/hooks.jsx#L678

Added line #L678 was not covered by tests
}
}
return null;

Check warning on line 681 in src/components/course/data/hooks.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/course/data/hooks.jsx#L681

Added line #L681 was not covered by tests
};

const coursePrice = getCourseRunPrice({
Expand Down
Loading
Loading