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 #319

Merged
merged 5 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
440 changes: 281 additions & 159 deletions package-lock.json

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
],
"dependencies": {
"@edx/brand": "npm:@edx/brand-openedx@^1.2.0",
"@edx/frontend-component-footer": "12.1.0",
"@edx/frontend-component-header": "4.3.0",
"@edx/frontend-platform": "4.6.0",
"@edx/frontend-component-footer": "12.2.0",
"@edx/frontend-component-header": "4.6.0",
"@edx/frontend-platform": "5.0.0",
"@edx/paragon": "20.45.0",
"@edx/react-unit-test-utils": "1.7.0",
"@edx/reactifex": "^2.1.1",
Expand All @@ -54,8 +54,8 @@
"react-dom": "17.0.2",
"react-helmet": "^6.1.0",
"react-redux": "^7.2.9",
"react-router": "5.2.0",
"react-router-dom": "5.2.0",
"react-router": "6.15.0",
"react-router-dom": "6.15.0",
"react-router-redux": "^5.0.0-alpha.9",
"redux": "4.0.5",
"redux-beacon": "^2.1.0",
Expand Down
29 changes: 13 additions & 16 deletions src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { BrowserRouter as Router, Route, Switch } from 'react-router-dom';
import { Route, Routes } from 'react-router-dom';

import { AppProvider } from '@edx/frontend-platform/react';

Expand All @@ -15,21 +15,18 @@ import Head from './head/Head';
const App = () => (
<AppProvider store={store}>
<Head />
<Router>
<div>
<Header />
<main>
<Switch>
<Route
exact
path={routePath}
component={GradebookPage}
/>
</Switch>
</main>
<Footer logo={process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG} />
</div>
</Router>
<div>
<Header />
<main>
<Routes>
<Route
path={routePath}
element={<GradebookPage />}
/>
</Routes>
</main>
<Footer logo={process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG} />
</div>
</AppProvider>
);

Expand Down
23 changes: 11 additions & 12 deletions src/App.test.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { shallow } from 'enzyme';

import { BrowserRouter as Router, Route, Switch } from 'react-router-dom';
import { Route, Routes } from 'react-router-dom';
import { AppProvider } from '@edx/frontend-platform/react';

import Footer from '@edx/frontend-component-footer';
Expand All @@ -17,7 +17,6 @@ import Head from './head/Head';
jest.mock('react-router-dom', () => ({
BrowserRouter: () => 'BrowserRouter',
Route: () => 'Route',
Switch: () => 'Switch',
}));
jest.mock('@edx/frontend-platform/react', () => ({
AppProvider: () => 'AppProvider',
Expand All @@ -32,7 +31,7 @@ jest.mock('@edx/frontend-component-header', () => 'Header');

const logo = 'fakeLogo.png';
let el;
let router;
let secondChild;

describe('App router component', () => {
test('snapshot', () => {
Expand All @@ -42,7 +41,7 @@ describe('App router component', () => {
beforeEach(() => {
process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG = logo;
el = shallow(<App />);
router = el.childAt(1);
secondChild = el.childAt(1);
});
describe('AppProvider', () => {
test('AppProvider is the parent component, passed the redux store props', () => {
Expand All @@ -57,24 +56,24 @@ describe('App router component', () => {
});
describe('Router', () => {
test('second child of AppProvider', () => {
expect(router.type()).toBe(Router);
expect(secondChild.type()).toBe('div');
});
test('Header is above/outside-of the routing', () => {
expect(router.childAt(0).childAt(0).type()).toBe(Header);
expect(router.childAt(0).childAt(1).type()).toBe('main');
expect(secondChild.childAt(0).type()).toBe(Header);
expect(secondChild.childAt(1).type()).toBe('main');
});
test('Routing - GradebookPage is only route', () => {
expect(router.find('main')).toEqual(shallow(
expect(secondChild.find('main')).toEqual(shallow(
<main>
<Switch>
<Route exact path={routePath} component={GradebookPage} />
</Switch>
<Routes>
<Route path={routePath} element={<GradebookPage />} />
</Routes>
</main>,
));
});
});
test('Footer logo drawn from env variable', () => {
expect(router.find(Footer).props().logo).toEqual(logo);
expect(secondChild.find(Footer).props().logo).toEqual(logo);
});
});
});
27 changes: 12 additions & 15 deletions src/__snapshots__/App.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,17 @@ exports[`App router component snapshot 1`] = `
store="testStore"
>
<Head />
<BrowserRouter>
<div>
<Header />
<main>
<Switch>
<Route
component="GradebookPage"
exact={true}
path="/:courseId"
/>
</Switch>
</main>
<Footer />
</div>
</BrowserRouter>
<div>
<Header />
<main>
<Component>
<Route
element={<GradebookPage />}
path="/:courseId"
/>
</Component>
</main>
<Footer />
</div>
</AppProvider>
`;
23 changes: 10 additions & 13 deletions src/containers/GradebookPage/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import GradesView from 'components/GradesView';
import GradebookFilters from 'components/GradebookFilters';
import BulkManagementHistoryView from 'components/BulkManagementHistoryView';

import { withParams, withNavigate, withLocation } from '../../utils/hoc';

/**
* <GradebookPage />
* Top-level view for the Gradebook MFE.
Expand All @@ -28,10 +30,11 @@ export class GradebookPage extends React.Component {

componentDidMount() {
const urlQuery = queryString.parse(this.props.location.search);
this.props.initializeApp(this.props.match.params.courseId, urlQuery);
this.props.initializeApp(this.props.courseId, urlQuery);
}

updateQueryParams(queryParams) {
const { pathname } = this.props.location;
const parsed = queryString.parse(this.props.location.search);
Object.keys(queryParams).forEach((key) => {
if (queryParams[key]) {
Expand All @@ -40,7 +43,7 @@ export class GradebookPage extends React.Component {
delete parsed[key];
}
});
this.props.history.push(`?${queryString.stringify(parsed)}`);
this.props.navigate({ pathname, search: `?${queryString.stringify(parsed)}` });
}

render() {
Expand All @@ -60,18 +63,12 @@ export class GradebookPage extends React.Component {
}
}
GradebookPage.defaultProps = {
location: { search: '' },
location: { pathname: '/', search: '' },
};
GradebookPage.propTypes = {
history: PropTypes.shape({
push: PropTypes.func,
}).isRequired,
location: PropTypes.shape({ search: PropTypes.string }),
match: PropTypes.shape({
params: PropTypes.shape({
courseId: PropTypes.string,
}),
}).isRequired,
navigate: PropTypes.func.isRequired,
location: PropTypes.shape({ pathname: PropTypes.string, search: PropTypes.string }),
courseId: PropTypes.string.isRequired,
// redux
activeView: PropTypes.string.isRequired,
initializeApp: PropTypes.func.isRequired,
Expand All @@ -85,4 +82,4 @@ export const mapDispatchToProps = {
initializeApp: thunkActions.app.initialize,
};

export default connect(mapStateToProps, mapDispatchToProps)(GradebookPage);
export default connect(mapStateToProps, mapDispatchToProps)(withParams(withNavigate(withLocation(GradebookPage))));
11 changes: 6 additions & 5 deletions src/containers/GradebookPage/test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ describe('GradebookPage', () => {
let el;
const props = {
location: {
pathname: '/',
search: 'searchString',
},
match: { params: { courseId } },
courseId,
activeView: views.grades,
};
beforeEach(() => {
props.initializeApp = jest.fn();
props.history = { push: jest.fn() };
props.navigate = jest.fn();
});
test('snapshot - shows BulkManagementHistoryView if activeView === views.bulkManagementHistory', () => {
el = shallow(<GradebookPage {...props} activeView={views.bulkManagementHistory} />);
Expand Down Expand Up @@ -130,7 +131,7 @@ describe('GradebookPage', () => {
const val2 = 'VALTWO!!';
const args = { [newKey]: val1, [props.location.search]: val2 };
el.instance().updateQueryParams(args);
expect(props.history.push).toHaveBeenCalledWith(`?${queryString.stringify(args)}`);
expect(props.navigate).toHaveBeenCalledWith({ pathname: '/', search: `?${queryString.stringify(args)}` });
});
it('clears values for non-truthy values', () => {
queryString.parse.mockImplementation(key => ({ [key]: key }));
Expand All @@ -139,8 +140,8 @@ describe('GradebookPage', () => {
const val2 = false;
const args = { [newKey]: val1, [props.location.search]: val2 };
el.instance().updateQueryParams(args);
expect(props.history.push).toHaveBeenCalledWith(
`?${queryString.stringify({ [newKey]: val1 })}`,
expect(props.navigate).toHaveBeenCalledWith(
{ pathname: '/', search: `?${queryString.stringify({ [newKey]: val1 })}` },
);
});
});
Expand Down
24 changes: 24 additions & 0 deletions src/utils/hoc.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';

import { useLocation, useNavigate, useParams } from 'react-router-dom';

export const withParams = WrappedComponent => {
const WithParamsComponent = props => <WrappedComponent {...useParams()} {...props} />;
return WithParamsComponent;
};

export const withNavigate = WrappedComponent => {
const WithNavigateComponent = props => {
const navigate = useNavigate();
return <WrappedComponent navigate={navigate} {...props} />;
};
return WithNavigateComponent;
};

export const withLocation = WrappedComponent => {
const WithLocationComponent = props => {
const location = useLocation();
return <WrappedComponent location={location} {...props} />;
};
return WithLocationComponent;
};
38 changes: 38 additions & 0 deletions src/utils/hoc.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import React from 'react';
import { mount } from 'enzyme';

import { withLocation, withNavigate } from './hoc';

const mockedNavigator = jest.fn();

jest.mock('react-router-dom', () => ({
useNavigate: () => mockedNavigator,
useLocation: () => ({
pathname: '/current-location',
}),
}));

// eslint-disable-next-line react/prop-types
const MockComponent = ({ navigate, location }) => (
// eslint-disable-next-line react/button-has-type, react/prop-types
<button id="btn" onClick={() => navigate('/some-route')}>{location.pathname}</button>
);
const WrappedComponent = withNavigate(withLocation(MockComponent));

test('Provide Navigation to Component', () => {
const wrapper = mount(
<WrappedComponent />,
);
const btn = wrapper.find('#btn');
btn.simulate('click');

expect(mockedNavigator).toHaveBeenCalledWith('/some-route');
});

test('Provide Location object to Component', () => {
const wrapper = mount(
<WrappedComponent />,
);

expect(wrapper.find('#btn').text()).toContain('/current-location');
});
Loading