Start adopting React Router v6 API using compatibility layer

React Router v6 contains a number of breaking changes, however they have provided a helpful compatibility layer which allows us to start adopting the new APIs while still using v5. This means we don't have to update everything in one go (which would also require updating to React 18). Change implemented so far: - use hooks from new API, `useHistory` replaced by `useNavigate` - no standalone `Route` components, must be inside a `Switch` - use `CompatRoute` to begin consuming the new APIs - implement replacement for `useRouteMatch` from v5 which is not available in the new API. This also allows us to clean up the `NamespacedRoute` and related `namespacedMatch` used for navigation when switching namespaces avoiding the need to explicitly pass the `allNamespacesPath` on each namespaced resource details page.
tektoncd · Oct 27, 2022 · 9c354e9 · 9c354e9
1 parent d87019d
commit 9c354e9
Show file tree

Hide file tree

Showing 59 changed files with 625 additions and 559 deletions.
diff --git a/.storybook/Container.js b/.storybook/Container.js
@@ -14,7 +14,8 @@ limitations under the License.
 import React from 'react';
 import { IntlProvider } from 'react-intl';
 import { createMemoryHistory } from 'history';
-import { Router, Route } from 'react-router-dom';
+import { Router, Switch } from 'react-router-dom';
+import { CompatRoute, CompatRouter } from 'react-router-dom-v5-compat';
 
 import messages from '../src/nls/messages_en.json';
 
@@ -31,11 +32,15 @@ export default function Container({
       {notes && <p>{notes}</p>}
       <div data-floating-menu-container role="main">
         <Router history={createMemoryHistory({ initialEntries: [route] })}>
-          <Route path={path}>
-            <React.StrictMode>
-              <Story />
-            </React.StrictMode>
-          </Route>
+          <CompatRouter>
+            <Switch>
+              <CompatRoute path={path}>
+                <React.StrictMode>
+                  <Story />
+                </React.StrictMode>
+              </CompatRoute>
+            </Switch>
+          </CompatRouter>
         </Router>
       </div>
     </IntlProvider>

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -41,6 +41,7 @@
     "react-hot-loader": "^4.13.0",
     "react-intl": "^6.2.1",
     "react-router-dom": "^5.3.4",
+    "react-router-dom-v5-compat": "^6.4.2",
     "reconnecting-websocket": "^4.4.0"
   },
   "devDependencies": {

diff --git a/packages/components/src/components/PageErrorBoundary/PageErrorBoundary.js b/packages/components/src/components/PageErrorBoundary/PageErrorBoundary.js
@@ -13,7 +13,7 @@ limitations under the License.
 
 import React from 'react';
 import { useIntl } from 'react-intl';
-import { useLocation } from 'react-router-dom';
+import { useLocation } from 'react-router-dom-v5-compat';
 
 import { ErrorBoundary } from '..';
 

diff --git a/packages/components/src/utils/test.js b/packages/components/src/utils/test.js
@@ -1,5 +1,5 @@
 /*
-Copyright 2019-2021 The Tekton Authors
+Copyright 2019-2022 The Tekton Authors
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
@@ -12,18 +12,23 @@ limitations under the License.
 */
 /* istanbul ignore file */
 import React from 'react';
-import { BrowserRouter, Route } from 'react-router-dom';
+import { BrowserRouter, Switch } from 'react-router-dom';
+import { CompatRoute, CompatRouter } from 'react-router-dom-v5-compat';
 import { render as baseRender } from '@testing-library/react';
 import { IntlProvider } from 'react-intl';
 
 function RouterWrapper({ children, path }) {
   return (
     <BrowserRouter>
-      <Route path={path}>
-        <IntlProvider locale="en" defaultLocale="en" messages={{}}>
-          {children}
-        </IntlProvider>
-      </Route>
+      <CompatRouter>
+        <Switch>
+          <CompatRoute path={path}>
+            <IntlProvider locale="en" defaultLocale="en" messages={{}}>
+              {children}
+            </IntlProvider>
+          </CompatRoute>
+        </Switch>
+      </CompatRouter>
     </BrowserRouter>
   );
 }

diff --git a/packages/utils/src/utils/index.js b/packages/utils/src/utils/index.js
@@ -243,16 +243,16 @@ export function getFilters({ search }) {
   return filters;
 }
 
-export function getAddFilterHandler({ history, location }) {
+export function getAddFilterHandler({ location, navigate }) {
   return function handleAddFilter(labelFilters) {
     const queryParams = new URLSearchParams(location.search);
     queryParams.set('labelSelector', labelFilters);
     const browserURL = location.pathname.concat(`?${queryParams.toString()}`);
-    history.push(browserURL);
+    navigate(browserURL);
   };
 }
 
-export function getDeleteFilterHandler({ history, location }) {
+export function getDeleteFilterHandler({ location, navigate }) {
   return function handleDeleteFilter(filter) {
     const queryParams = new URLSearchParams(location.search);
     const labelFilters = queryParams.getAll('labelSelector');
@@ -269,16 +269,16 @@ export function getDeleteFilterHandler({ history, location }) {
     const browserURL = location.pathname.concat(
       queryString ? `?${queryString}` : ''
     );
-    history.push(browserURL);
+    navigate(browserURL);
   };
 }
 
-export function getClearFiltersHandler({ history, location }) {
+export function getClearFiltersHandler({ location, navigate }) {
   return function handleClearFilters() {
     const queryParams = new URLSearchParams(location.search);
     queryParams.delete('labelSelector');
     const browserURL = location.pathname.concat(`?${queryParams.toString()}`);
-    history.push(browserURL);
+    navigate(browserURL);
   };
 }
 
@@ -299,7 +299,7 @@ export function getStatusFilter({ search }) {
   return status;
 }
 
-export function getStatusFilterHandler({ history, location }) {
+export function getStatusFilterHandler({ location, navigate }) {
   return function setStatusFilter(statusFilter) {
     const queryParams = new URLSearchParams(location.search);
     if (!statusFilter) {
@@ -308,7 +308,7 @@ export function getStatusFilterHandler({ history, location }) {
       queryParams.set('status', statusFilter);
     }
     const browserURL = location.pathname.concat(`?${queryParams.toString()}`);
-    history.push(browserURL);
+    navigate(browserURL);
   };
 }
 

diff --git a/packages/utils/src/utils/index.test.js b/packages/utils/src/utils/index.test.js
@@ -218,12 +218,12 @@ it('getFilters', () => {
 
 it('getAddFilterHandler', () => {
   const url = 'someURL';
-  const history = { push: jest.fn() };
+  const navigate = jest.fn();
   const location = { pathname: url, search: '?nonFilterQueryParam=someValue' };
-  const handleAddFilter = getAddFilterHandler({ history, location });
+  const handleAddFilter = getAddFilterHandler({ location, navigate });
   const labelFilters = ['foo1=bar1', 'foo2=bar2'];
   handleAddFilter(labelFilters);
-  expect(history.push).toHaveBeenCalledWith(
+  expect(navigate).toHaveBeenCalledWith(
     `${url}?nonFilterQueryParam=someValue&labelSelector=${encodeURIComponent(
       'foo1=bar1,foo2=bar2'
     )}`
@@ -234,29 +234,29 @@ describe('getDeleteFilterHandler', () => {
   it('should redirect to unfiltered URL if no filters remain', () => {
     const search = `?labelSelector=${encodeURIComponent('foo=bar')}`;
     const url = 'someURL';
-    const history = { push: jest.fn() };
+    const navigate = jest.fn();
     const location = { pathname: url, search };
     const handleDeleteFilter = getDeleteFilterHandler({
-      history,
-      location
+      location,
+      navigate
     });
     handleDeleteFilter('foo=bar');
-    expect(history.push).toHaveBeenCalledWith(url);
+    expect(navigate).toHaveBeenCalledWith(url);
   });
 
   it('should correctly remove a filter from the URL', () => {
     const search = `?labelSelector=${encodeURIComponent(
       'foo1=bar1,foo2=bar2'
     )}`;
     const url = 'someURL';
-    const history = { push: jest.fn() };
+    const navigate = jest.fn();
     const location = { pathname: url, search };
     const handleDeleteFilter = getDeleteFilterHandler({
-      history,
-      location
+      location,
+      navigate
     });
     handleDeleteFilter('foo1=bar1');
-    expect(history.push).toHaveBeenCalledWith(
+    expect(navigate).toHaveBeenCalledWith(
       `${url}?labelSelector=${encodeURIComponent('foo2=bar2')}`
     );
   });
@@ -490,14 +490,14 @@ describe('getStatusFilter', () => {
       'foo1=bar1,foo2=bar2'
     )}`;
     const url = 'someURL';
-    const history = { push: jest.fn() };
+    const navigate = jest.fn();
     const location = { pathname: url, search };
     const handleDeleteFilter = getDeleteFilterHandler({
-      history,
-      location
+      location,
+      navigate
     });
     handleDeleteFilter('foo1=bar1');
-    expect(history.push).toHaveBeenCalledWith(
+    expect(navigate).toHaveBeenCalledWith(
       `${url}?labelSelector=${encodeURIComponent('foo2=bar2')}`
     );
   });
@@ -507,47 +507,47 @@ describe('getStatusFilterHandler', () => {
   it('should redirect to unfiltered URL if no status specified', () => {
     const search = '?nonFilterQueryParam=someValue&status=cancelled';
     const url = 'someURL';
-    const history = { push: jest.fn() };
+    const navigate = jest.fn();
     const location = { pathname: url, search };
     const setStatusFilter = getStatusFilterHandler({
-      history,
-      location
+      location,
+      navigate
     });
     setStatusFilter('');
-    expect(history.push).toHaveBeenCalledWith(
+    expect(navigate).toHaveBeenCalledWith(
       `${url}?nonFilterQueryParam=someValue`
     );
   });
 
   it('should set a valid status filter in the URL', () => {
     const url = 'someURL';
-    const history = { push: jest.fn() };
+    const navigate = jest.fn();
     const location = {
       pathname: url,
       search: '?nonFilterQueryParam=someValue'
     };
     const setStatusFilter = getStatusFilterHandler({
-      history,
-      location
+      location,
+      navigate
     });
     const statusFilter = 'cancelled';
     setStatusFilter(statusFilter);
-    expect(history.push).toHaveBeenCalledWith(
+    expect(navigate).toHaveBeenCalledWith(
       `${url}?nonFilterQueryParam=someValue&status=${statusFilter}`
     );
   });
 
   it('should update the status filter in the URL', () => {
     const url = 'someURL';
-    const history = { push: jest.fn() };
+    const navigate = jest.fn();
     const location = { pathname: url, search: '?status=cancelled' };
     const setStatusFilter = getStatusFilterHandler({
-      history,
-      location
+      location,
+      navigate
     });
     const statusFilter = 'completed';
     setStatusFilter(statusFilter);
-    expect(history.push).toHaveBeenCalledWith(`${url}?status=${statusFilter}`);
+    expect(navigate).toHaveBeenCalledWith(`${url}?status=${statusFilter}`);
   });
 });
 

diff --git a/packages/utils/src/utils/router.js b/packages/utils/src/utils/router.js
@@ -10,7 +10,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-import { generatePath } from 'react-router-dom';
+import { generatePath } from 'react-router-dom-v5-compat';
 
 function byNamespace({ path = '' } = {}) {
   return `/namespaces/:namespace${path}`;