Skip to content

Commit

Permalink
Update route definitions to prepare for react-router update
Browse files Browse the repository at this point in the history 
The next release of react-router (v6) makes some changes to the way
routes are defined and behave. The `render` and `component` props
should no longer be used in v5.1+ to prepare for this change, instead
favouring `children`.

Instead of passing through the `location` and `match` props from the
`Route`, we should instead retrieve them when / where needed via the
hooks provided by `react-router`, i.e. `useLocation`, `useParams`, etc.
  • Loading branch information
@AlanGreene @tekton-robot
AlanGreene authored and tekton-robot committed Sep 27, 2021
1 parent 7a537e4 commit 3e96d84
Show file tree
Hide file tree
Showing 64 changed files with 656 additions and 1,268 deletions.
4 changes: 2 additions & 2 deletions .storybook/Container.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2019-2020 The Tekton Authors
Copyright 2019-2021 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
Expand Down Expand Up @@ -28,7 +28,7 @@ export default function Container({ story, notes }) {
{notes && <p>{notes}</p>}
<div data-floating-menu-container role="main">
<Router history={history}>
<Route path="/" component={() => story()} />
<Route path="/">{() => story()}</Route>
</Router>
</div>
</IntlProvider>
Expand Down
9 changes: 5 additions & 4 deletions packages/components/src/components/PageErrorBoundary/PageErrorBoundary.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2019 The Tekton Authors
Copyright 2019-2021 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
Expand All @@ -13,11 +13,12 @@ limitations under the License.

import React from 'react';
import { injectIntl } from 'react-intl';
import { withRouter } from 'react-router';
import { useLocation } from 'react-router-dom';

import { ErrorBoundary } from '..';

const PageErrorBoundary = ({ children, intl, location }) => {
const PageErrorBoundary = ({ children, intl }) => {
const location = useLocation();
const message = intl.formatMessage({
id: 'dashboard.errorBoundary.pageError',
defaultMessage: 'Error loading page'
Expand All @@ -30,4 +31,4 @@ const PageErrorBoundary = ({ children, intl, location }) => {
);
};

export default withRouter(injectIntl(PageErrorBoundary));
export default injectIntl(PageErrorBoundary);
15 changes: 9 additions & 6 deletions packages/components/src/utils/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,26 @@ limitations under the License.
*/
/* istanbul ignore file */
import React from 'react';
import { BrowserRouter } from 'react-router-dom';
import { BrowserRouter, Route } from 'react-router-dom';
import { render as baseRender } from '@testing-library/react';
import { IntlProvider } from 'react-intl';

function RouterWrapper({ children }) {
function RouterWrapper({ children, path }) {
return (
<BrowserRouter>
<IntlProvider locale="en" defaultLocale="en" messages={{}}>
{children}
</IntlProvider>
<Route path={path}>
<IntlProvider locale="en" defaultLocale="en" messages={{}}>
{children}
</IntlProvider>
</Route>
</BrowserRouter>
);
}

export function renderWithRouter(
ui,
{
path = '/',
rerender,
route = '/',
wrapper: Wrapper = React.Fragment,
Expand All @@ -41,7 +44,7 @@ export function renderWithRouter(
route,
wrapper: ({ children }) => (
<Wrapper>
<RouterWrapper>{children}</RouterWrapper>
<RouterWrapper path={path}>{children}</RouterWrapper>
</Wrapper>
),
...otherOptions
Expand Down
18 changes: 10 additions & 8 deletions packages/utils/src/utils/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,16 @@ export function getFilters({ search }) {
return filters;
}

export function getAddFilterHandler({ history, location, match }) {
export function getAddFilterHandler({ history, location }) {
return function handleAddFilter(labelFilters) {
const queryParams = new URLSearchParams(location.search);
queryParams.set('labelSelector', labelFilters);
const browserURL = match.url.concat(`?${queryParams.toString()}`);
const browserURL = location.pathname.concat(`?${queryParams.toString()}`);
history.push(browserURL);
};
}

export function getDeleteFilterHandler({ history, location, match }) {
export function getDeleteFilterHandler({ history, location }) {
return function handleDeleteFilter(filter) {
const queryParams = new URLSearchParams(location.search);
const labelFilters = queryParams.getAll('labelSelector');
Expand All @@ -190,16 +190,18 @@ export function getDeleteFilterHandler({ history, location, match }) {
queryParams.set('labelSelector', labelFiltersArray);
}
const queryString = queryParams.toString();
const browserURL = match.url.concat(queryString ? `?${queryString}` : '');
const browserURL = location.pathname.concat(
queryString ? `?${queryString}` : ''
);
history.push(browserURL);
};
}

export function getClearFiltersHandler({ history, location, match }) {
export function getClearFiltersHandler({ history, location }) {
return function handleClearFilters() {
const queryParams = new URLSearchParams(location.search);
queryParams.delete('labelSelector');
const browserURL = match.url.concat(`?${queryParams.toString()}`);
const browserURL = location.pathname.concat(`?${queryParams.toString()}`);
history.push(browserURL);
};
}
Expand All @@ -221,15 +223,15 @@ export function getStatusFilter({ search }) {
return status;
}

export function getStatusFilterHandler({ history, location, match }) {
export function getStatusFilterHandler({ history, location }) {
return function setStatusFilter(statusFilter) {
const queryParams = new URLSearchParams(location.search);
if (!statusFilter) {
queryParams.delete('status');
} else {
queryParams.set('status', statusFilter);
}
const browserURL = match.url.concat(`?${queryParams.toString()}`);
const browserURL = location.pathname.concat(`?${queryParams.toString()}`);
history.push(browserURL);
};
}
Expand Down
44 changes: 17 additions & 27 deletions packages/utils/src/utils/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,8 @@ it('getFilters', () => {
it('getAddFilterHandler', () => {
const url = 'someURL';
const history = { push: jest.fn() };
const location = { search: '?nonFilterQueryParam=someValue' };
const match = { url };
const handleAddFilter = getAddFilterHandler({ history, location, match });
const location = { pathname: url, search: '?nonFilterQueryParam=someValue' };
const handleAddFilter = getAddFilterHandler({ history, location });
const labelFilters = ['foo1=bar1', 'foo2=bar2'];
handleAddFilter(labelFilters);
expect(history.push).toHaveBeenCalledWith(
Expand All @@ -222,12 +221,10 @@ describe('getDeleteFilterHandler', () => {
const search = `?labelSelector=${encodeURIComponent('foo=bar')}`;
const url = 'someURL';
const history = { push: jest.fn() };
const location = { search };
const match = { url };
const location = { pathname: url, search };
const handleDeleteFilter = getDeleteFilterHandler({
history,
location,
match
location
});
handleDeleteFilter('foo=bar');
expect(history.push).toHaveBeenCalledWith(url);
Expand All @@ -239,12 +236,10 @@ describe('getDeleteFilterHandler', () => {
)}`;
const url = 'someURL';
const history = { push: jest.fn() };
const location = { search };
const match = { url };
const location = { pathname: url, search };
const handleDeleteFilter = getDeleteFilterHandler({
history,
location,
match
location
});
handleDeleteFilter('foo1=bar1');
expect(history.push).toHaveBeenCalledWith(
Expand Down Expand Up @@ -430,12 +425,10 @@ describe('getStatusFilter', () => {
)}`;
const url = 'someURL';
const history = { push: jest.fn() };
const location = { search };
const match = { url };
const location = { pathname: url, search };
const handleDeleteFilter = getDeleteFilterHandler({
history,
location,
match
location
});
handleDeleteFilter('foo1=bar1');
expect(history.push).toHaveBeenCalledWith(
Expand All @@ -449,12 +442,10 @@ describe('getStatusFilterHandler', () => {
const search = '?nonFilterQueryParam=someValue&status=cancelled';
const url = 'someURL';
const history = { push: jest.fn() };
const location = { search };
const match = { url };
const location = { pathname: url, search };
const setStatusFilter = getStatusFilterHandler({
history,
location,
match
location
});
setStatusFilter('');
expect(history.push).toHaveBeenCalledWith(
Expand All @@ -465,12 +456,13 @@ describe('getStatusFilterHandler', () => {
it('should set a valid status filter in the URL', () => {
const url = 'someURL';
const history = { push: jest.fn() };
const location = { search: '?nonFilterQueryParam=someValue' };
const match = { url };
const location = {
pathname: url,
search: '?nonFilterQueryParam=someValue'
};
const setStatusFilter = getStatusFilterHandler({
history,
location,
match
location
});
const statusFilter = 'cancelled';
setStatusFilter(statusFilter);
Expand All @@ -482,12 +474,10 @@ describe('getStatusFilterHandler', () => {
it('should update the status filter in the URL', () => {
const url = 'someURL';
const history = { push: jest.fn() };
const location = { search: '?status=cancelled' };
const match = { url };
const location = { pathname: url, search: '?status=cancelled' };
const setStatusFilter = getStatusFilterHandler({
history,
location,
match
location
});
const statusFilter = 'completed';
setStatusFilter(statusFilter);
Expand Down
Loading

0 comments on commit 3e96d84

Please sign in to comment.