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

[v6] Add location prop to Routes and RouteOptions to useRoutes #7571

Closed
wants to merge 2 commits into from
Closed
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
62 changes: 37 additions & 25 deletions packages/react-router/__tests__/Routes-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@ import { create as createTestRenderer } from 'react-test-renderer';
import { MemoryRouter as Router, Routes, Route } from 'react-router';

describe('A <Routes>', () => {
it('renders the first route that matches the URL', () => {
function Home() {
return <h1>Home</h1>;
}
function Home() {
return <h1>Home</h1>;
}

function Admin() {
return <h1>Admin</h1>;
}

it('renders the first route that matches the URL', () => {
let renderer = createTestRenderer(
<Router initialEntries={['/']}>
<Routes>
Expand All @@ -20,19 +24,11 @@ describe('A <Routes>', () => {
});

it('does not render a 2nd route that also matches the URL', () => {
function Home() {
return <h1>Home</h1>;
}

function Dashboard() {
return <h1>Dashboard</h1>;
}

let renderer = createTestRenderer(
<Router initialEntries={['/home']}>
<Routes>
<Route path="/home" element={<Home />} />
<Route path="/home" element={<Dashboard />} />
<Route path="/home" element={<Admin />} />
</Routes>
</Router>
);
Expand All @@ -41,10 +37,6 @@ describe('A <Routes>', () => {
});

it('renders with non-element children', () => {
function Home() {
return <h1>Home</h1>;
}

let renderer = createTestRenderer(
<Router initialEntries={['/']}>
<Routes>
Expand All @@ -59,14 +51,6 @@ describe('A <Routes>', () => {
});

it('renders with React.Fragment children', () => {
function Home() {
return <h1>Home</h1>;
}

function Admin() {
return <h1>Admin</h1>;
}

let renderer = createTestRenderer(
<Router initialEntries={['/admin']}>
<Routes>
Expand All @@ -80,4 +64,32 @@ describe('A <Routes>', () => {

expect(renderer.toJSON()).toMatchSnapshot();
});

it('renders route paths prefixed by a basename', () => {
let renderer = createTestRenderer(
<Router initialEntries={['/parent']}>
<Routes basename="/parent">
<Route path="/" element={<Home />} />
</Routes>
</Router>
);

expect(renderer.toJSON()).toMatchSnapshot();
});

it('renders routes for a different location', () => {
let location = {
pathname: '/home'
};

let renderer = createTestRenderer(
<Router initialEntries={['/']}>
<Routes location={location}>
<Route path="/home" element={<Home />} />
</Routes>
</Router>
);

expect(renderer.toJSON()).toMatchSnapshot();
});
});
12 changes: 12 additions & 0 deletions packages/react-router/__tests__/__snapshots__/Routes-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ exports[`A <Routes> does not render a 2nd route that also matches the URL 1`] =
</h1>
`;

exports[`A <Routes> renders route paths prefixed by a basename 1`] = `
<h1>
Home
</h1>
`;

exports[`A <Routes> renders routes for a different location 1`] = `
<h1>
Home
</h1>
`;

exports[`A <Routes> renders the first route that matches the URL 1`] = `
<h1>
Home
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`useRoutes accepts \`basename\` as optional parameter 1`] = `
<h1>
Home
</h1>
`;

exports[`useRoutes accepts \`location\` as optional parameter 1`] = `
<h1>
About
</h1>
`;

exports[`useRoutes returns the matching element from a route config 1`] = `
<h1>
Home
Expand Down
54 changes: 50 additions & 4 deletions packages/react-router/__tests__/useRoutes-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,63 @@ import { create as createTestRenderer } from 'react-test-renderer';
import { MemoryRouter as Router, useRoutes } from 'react-router';

describe('useRoutes', () => {
function Home() {
return <h1>Home</h1>;
}

function About() {
return <h1>About</h1>;
}

it('returns the matching element from a route config', () => {
function RoutesRenderer({ routes }) {
return useRoutes(routes);
}

function Home() {
return <h1>Home</h1>;
let routes = [
{ path: 'home', element: <Home /> },
{ path: 'about', element: <About /> }
];

let renderer = createTestRenderer(
<Router initialEntries={['/home']}>
<RoutesRenderer routes={routes} />
</Router>
);

expect(renderer.toJSON()).toMatchSnapshot();
});

it('accepts `basename` as optional parameter', () => {
function RoutesRenderer({ routes }) {
return useRoutes(routes, {
basename: '/parent'
});
}

function About() {
return <h1>About</h1>;
let routes = [
{ path: 'home', element: <Home /> },
{ path: 'about', element: <About /> }
];

let renderer = createTestRenderer(
<Router initialEntries={['/parent/home']}>
<RoutesRenderer routes={routes} />
</Router>
);

expect(renderer.toJSON()).toMatchSnapshot();
});

it('accepts `location` as optional parameter', () => {
let location = {
pathname: '/about'
};

function RoutesRenderer({ routes }) {
return useRoutes(routes, {
location: location
});
}

let routes = [
Expand Down
34 changes: 19 additions & 15 deletions packages/react-router/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,7 @@ function warning(cond: boolean, message: string): void {
}

const alreadyWarned: Record<string, boolean> = {};
function warningOnce(
key: string,
cond: boolean,
message: string
) {
function warningOnce(key: string, cond: boolean, message: string) {
if (!cond && !alreadyWarned[key]) {
alreadyWarned[key] = true;
warning(false, message);
Expand Down Expand Up @@ -332,14 +328,16 @@ if (__DEV__) {
*/
export function Routes({
basename = '',
location,
children
}: RoutesProps): React.ReactElement | null {
let routes = createRoutesFromChildren(children);
return useRoutes_(routes, basename);
return useRoutes_(routes, { basename, location });
}

export interface RoutesProps {
basename?: string;
location?: Location;
children?: React.ReactNode;
}

Expand Down Expand Up @@ -553,6 +551,11 @@ export function useResolvedPath(to: To): Path {
return React.useMemo(() => resolvePath(to, pathname), [to, pathname]);
}

interface RoutesOptions {
basename?: string;
location?: Location;
}

/**
* Returns the element of the route that matched the current location, prepared
* with the correct context to render the remainder of the route tree. Route
Expand All @@ -563,7 +566,7 @@ export function useResolvedPath(to: To): Path {
*/
export function useRoutes(
partialRoutes: PartialRouteObject[],
basename = ''
{ basename = '', location }: RoutesOptions = {}
): React.ReactElement | null {
invariant(
useInRouterContext(),
Expand All @@ -576,12 +579,12 @@ export function useRoutes(
partialRoutes
]);

return useRoutes_(routes, basename);
return useRoutes_(routes, { basename, location });
}

function useRoutes_(
routes: RouteObject[],
basename = ''
{ basename = '', location }: RoutesOptions = {}
): React.ReactElement | null {
let {
route: parentRoute,
Expand All @@ -608,12 +611,13 @@ function useRoutes_(

basename = basename ? joinPaths([parentPathname, basename]) : parentPathname;

let location = useLocation() as Location;
let matches = React.useMemo(() => matchRoutes(routes, location, basename), [
location,
routes,
basename
]);
let currentLocation = useLocation() as Location;
let usedLocation = location || currentLocation;

let matches = React.useMemo(
() => matchRoutes(routes, usedLocation, basename),
[location, routes, basename]
Copy link

@kdembler kdembler Feb 1, 2021

Choose a reason for hiding this comment

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

useMemo dependencies weren't updated here with usedLocation

);

if (!matches) {
// TODO: Warn about nothing matching, suggest using a catch-all route.
Expand Down