Skip to content

Commit cf09a94

Browse files
feat(content-sidebar): remove react-router-dom in favor of custom router
Co-Authored-By: tjiang@box.com <tjiang@box.com>
1 parent c5d6dca commit cf09a94

26 files changed

+1028
-696
lines changed
Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,66 @@
11
import * as React from 'react';
2-
import { MemoryRouter, Router } from 'react-router';
3-
import { History } from 'history';
2+
import { createBrowserHistory, createMemoryHistory, History, Location } from 'history';
43

5-
type Props = {
4+
export interface NavRouterContextValue {
5+
history: History;
6+
location: Location;
7+
match: {
8+
params: Record<string, string>;
9+
isExact: boolean;
10+
path: string;
11+
url: string;
12+
};
13+
}
14+
15+
export interface Props {
616
children: React.ReactNode;
717
history?: History;
8-
initialEntries?: History.LocationDescriptor[];
9-
};
18+
initialEntries?: string[];
19+
}
1020

11-
const NavRouter = ({ children, history, ...rest }: Props) => {
12-
if (history) {
13-
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
14-
// @ts-ignore
15-
return <Router history={history}>{children}</Router>;
21+
export const NavRouterContext = React.createContext<NavRouterContextValue | null>(null);
22+
23+
export const useNavRouter = () => {
24+
const context = React.useContext(NavRouterContext);
25+
if (!context) {
26+
throw new Error('useNavRouter must be used within a NavRouter');
1627
}
28+
return context;
29+
};
30+
31+
const NavRouter = ({ children, history: providedHistory, initialEntries }: Props) => {
32+
const [history] = React.useState<History>(
33+
() => providedHistory || (initialEntries ? createMemoryHistory({ initialEntries }) : createBrowserHistory()),
34+
);
35+
const [location, setLocation] = React.useState<Location>(history.location);
36+
37+
React.useEffect(() => {
38+
const unlisten = history.listen(update => {
39+
setLocation(update);
40+
});
41+
return unlisten;
42+
}, [history]);
43+
44+
const match = React.useMemo(() => {
45+
const path = location.pathname;
46+
return {
47+
path,
48+
url: path,
49+
isExact: path === location.pathname,
50+
params: {},
51+
};
52+
}, [location]);
53+
54+
const value = React.useMemo(
55+
() => ({
56+
history,
57+
location,
58+
match,
59+
}),
60+
[history, location, match],
61+
);
1762

18-
return <MemoryRouter {...rest}>{children}</MemoryRouter>;
63+
return <NavRouterContext.Provider value={value}>{children}</NavRouterContext.Provider>;
1964
};
2065

2166
export default NavRouter;
Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,40 @@
11
import * as React from 'react';
2-
import { shallow } from 'enzyme';
32
import { createMemoryHistory } from 'history';
3+
import { render, screen } from '@testing-library/react';
44
import NavRouter from '../NavRouter';
55
import withNavRouter from '../withNavRouter';
6-
import { WithNavRouterProps } from '../types';
76

87
type Props = {
98
value?: string;
109
};
1110

12-
describe('src/eleemnts/common/nav-router/withNavRouter', () => {
13-
const TestComponent = ({ value }: Props) => <div>{`Test ${value}`}</div>;
11+
describe('src/elements/common/nav-router/withNavRouter', () => {
12+
const TestComponent = ({ value, location }: Props & { location?: { pathname: string } }) => (
13+
<div>{`Test ${value} ${location?.pathname || ''}`}</div>
14+
);
1415
const WrappedComponent = withNavRouter(TestComponent);
1516

16-
const getWrapper = (props?: Props & WithNavRouterProps) => shallow(<WrappedComponent {...props} />);
17-
18-
test('should wrap component with NavRouter', () => {
19-
const wrapper = getWrapper();
20-
21-
expect(wrapper.find(NavRouter)).toBeTruthy();
22-
expect(wrapper.find(TestComponent)).toBeTruthy();
23-
});
24-
25-
test('should provide the appropriate props to NavRouter and the wrapped component', () => {
17+
const renderComponent = (props?: Props) => {
2618
const history = createMemoryHistory();
27-
const initialEntries = ['foo'];
28-
const value = 'foo';
29-
const wrapper = getWrapper({
30-
history,
31-
initialEntries,
32-
value,
33-
});
34-
35-
const navRouter = wrapper.find(NavRouter);
36-
expect(navRouter.prop('history')).toEqual(history);
37-
expect(navRouter.prop('initialEntries')).toEqual(initialEntries);
19+
return render(
20+
<NavRouter history={history}>
21+
<WrappedComponent {...props} />
22+
</NavRouter>,
23+
);
24+
};
25+
26+
test('should render wrapped component with router context', () => {
27+
renderComponent({ value: 'test' });
28+
expect(screen.getByText(/Test test/)).toBeInTheDocument();
29+
});
3830

39-
expect(wrapper.find(TestComponent).prop('value')).toEqual(value);
31+
test('should provide router props to wrapped component', () => {
32+
const history = createMemoryHistory({ initialEntries: ['/test-path'] });
33+
render(
34+
<NavRouter history={history}>
35+
<WrappedComponent value="test" />
36+
</NavRouter>,
37+
);
38+
expect(screen.getByText(/Test test \/test-path/)).toBeInTheDocument();
4039
});
4140
});
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
export { default } from './NavRouter';
1+
export { default as NavRouter, useNavRouter, NavRouterContext, type NavRouterContextValue } from './NavRouter';
22
export { default as withNavRouter } from './withNavRouter';
33
export * from './types';
Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1-
import { History } from 'history';
1+
import { History, Location } from 'history';
22

3-
export type WithNavRouterProps = {
4-
history?: History;
5-
initialEntries?: History.LocationDescriptor[];
6-
};
3+
export interface WithNavRouterProps {
4+
history: History;
5+
location: Location;
6+
match: {
7+
params: Record<string, string>;
8+
isExact: boolean;
9+
path: string;
10+
url: string;
11+
};
12+
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { History } from 'history';
2+
import { WithNavRouterProps } from './types';
3+
4+
export interface MatchResult {
5+
isExact: boolean;
6+
params: Record<string, string>;
7+
}
8+
9+
export function matchPath(pathname: string, path: string | string[]): MatchResult {
10+
const paths = Array.isArray(path) ? path : [path];
11+
const params: Record<string, string> = {};
12+
13+
for (const currentPath of paths) {
14+
const pathSegments = currentPath.split('/').filter(Boolean);
15+
const pathnameSegments = pathname.split('/').filter(Boolean);
16+
17+
if (pathSegments.length <= pathnameSegments.length) {
18+
let isMatch = true;
19+
const segmentLength = pathSegments.length;
20+
21+
for (let i = 0; i < segmentLength; i += 1) {
22+
const pathSegment = pathSegments[i];
23+
const pathnameSegment = pathnameSegments[i];
24+
25+
if (pathSegment.startsWith(':')) {
26+
// This is a parameter
27+
const paramName = pathSegment.slice(1);
28+
params[paramName] = pathnameSegment;
29+
} else if (pathSegment === '*') {
30+
// Wildcard matches anything
31+
params[i.toString()] = pathnameSegments.slice(i).join('/');
32+
break;
33+
} else if (pathSegment !== pathnameSegment) {
34+
isMatch = false;
35+
break;
36+
}
37+
}
38+
39+
if (isMatch) {
40+
return {
41+
isExact: pathSegments.length === pathnameSegments.length,
42+
params,
43+
};
44+
}
45+
}
46+
}
47+
48+
return { isExact: false, params: {} };
49+
}
50+
51+
export function useMatch(
52+
props: Partial<WithNavRouterProps> & { history: History },
53+
path: string | string[],
54+
): MatchResult {
55+
const location = props.location || props.history.location;
56+
return matchPath(location.pathname, path);
57+
}
Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
import * as React from 'react';
2-
import NavRouter from './NavRouter';
3-
import { WithNavRouterProps } from './types';
2+
import { useNavRouter } from './NavRouter';
3+
import type { WithNavRouterProps } from './types';
44

5-
const withNavRouter = <P extends object>(Component: React.ComponentType<P>): React.FC<P & WithNavRouterProps> => {
6-
function WithNavRouter({ history, initialEntries, ...rest }: P & WithNavRouterProps) {
7-
return (
8-
<NavRouter history={history} initialEntries={initialEntries}>
9-
<Component {...(rest as P)} />
10-
</NavRouter>
11-
);
12-
}
5+
const withNavRouter = <P extends object>(
6+
WrappedComponent: React.ComponentType<P>,
7+
): React.FC<Omit<P, keyof WithNavRouterProps>> => {
8+
const WithNavRouterComponent: React.FC<Omit<P, keyof WithNavRouterProps>> = props => {
9+
const routerProps = useNavRouter();
10+
return <WrappedComponent {...(props as P)} {...routerProps} />;
11+
};
1312

14-
WithNavRouter.displayName = `withNavRouter(${Component.displayName || Component.name || 'Component'}`;
13+
WithNavRouterComponent.displayName = `WithNavRouter(${
14+
WrappedComponent.displayName || WrappedComponent.name || 'Component'
15+
})`;
1516

16-
return WithNavRouter;
17+
return WithNavRouterComponent;
1718
};
1819

1920
export default withNavRouter;

src/elements/content-explorer/__tests__/PreviewDialog.test.tsx

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,14 @@ import APICache from '../../../utils/Cache';
66
import PreviewDialog, { PreviewDialogProps } from '../PreviewDialog';
77

88
jest.mock('react-modal', () => {
9-
return jest.fn(({ children }) => <div aria-label="Preview">{children}</div>);
9+
return jest.fn(({ children, onRequestClose }) => (
10+
<div aria-label="Preview">
11+
<button aria-label="Close" onClick={onRequestClose}>
12+
Close
13+
</button>
14+
{children}
15+
</div>
16+
));
1017
});
1118

1219
describe('elements/content-explorer/PreviewDialog', () => {
@@ -41,10 +48,13 @@ describe('elements/content-explorer/PreviewDialog', () => {
4148
});
4249

4350
test('calls onCancel when modal is closed', async () => {
44-
renderComponent();
51+
const onCancel = jest.fn();
52+
renderComponent({ onCancel });
4553
const closeButton = screen.getByRole('button', { name: 'Close' });
54+
// Ensure we have the right close button by checking its parent
55+
expect(closeButton.closest('.bcpr-PreviewHeader-button-close')).toBeTruthy();
4656
await userEvent.click(closeButton);
47-
expect(defaultProps.onCancel).toHaveBeenCalled();
57+
expect(onCancel).toHaveBeenCalled();
4858
});
4959

5060
test('does not render when item is null', () => {

src/elements/content-preview/ContentPreview.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@ import setProp from 'lodash/set';
1717
import throttle from 'lodash/throttle';
1818
import uniqueid from 'lodash/uniqueId';
1919
import Measure from 'react-measure';
20-
import { withRouter } from 'react-router-dom';
21-
import type { ContextRouter } from 'react-router-dom';
20+
import { withNavRouter } from '../common/nav-router';
21+
import type { WithNavRouterProps } from '../common/nav-router/types';
2222
import { decode } from '../../utils/keys';
2323
import makeResponsive from '../common/makeResponsive';
24-
import { withNavRouter } from '../common/nav-router';
2524
import Internationalize from '../common/Internationalize';
2625
import AsyncLoad from '../common/async-load';
2726
import TokenService from '../../utils/TokenService';
@@ -129,7 +128,7 @@ type Props = {
129128
WithLoggerProps &
130129
WithAnnotationsProps &
131130
WithAnnotatorContextProps &
132-
ContextRouter;
131+
WithNavRouterProps;
133132

134133
type State = {
135134
canPrint?: boolean,
@@ -1407,7 +1406,7 @@ export default flow([
14071406
makeResponsive,
14081407
withAnnotatorContext,
14091408
withAnnotations,
1410-
withRouter,
1409+
14111410
withNavRouter,
14121411
withFeatureConsumer,
14131412
withFeatureProvider,

0 commit comments

Comments
 (0)