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

Fix pathless route's match when parent is null #5964

Merged
merged 6 commits into from
Feb 22, 2018
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
25 changes: 25 additions & 0 deletions packages/react-router/docs/api/match.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,28 @@ You'll have access `match` objects in various places:

If a Route does not have a `path`, and therefore always matches, you'll get the closest parent match. Same goes for `withRouter`.

## null matches

A `<Route>` that uses the `children` prop will call its `children` function even when the route's `path` does not match the current location. When this is the case, the `match` will be `null`. Being able to render a `<Route>`'s contents when it does match can be useful, but certain challenges arise from this situation.

The default way to "resolve" URLs is to join the `match.url` string to the "relative" path.

```js
`${match.url}/relative-path`
```

If you attempt to do this when the match is `null`, you will end up with a `TypeError`. This means that it is considered unsafe to attempt to join "relative" paths inside of a `<Route>` when using the `children` prop.

A similar, but more subtle situation occurs when you use a pathless `<Route>` inside of a `<Route>` that generates a `null` match object.

```js
// location.pathname = '/matches'
<Route path='/does-not-match' children={({ match }) => (
// match === null
<Route render={({ match:pathlessMatch }) => (
// pathlessMatch === ???
)}/>
)}/>
```

Pathless `<Route>`s inherit their `match` object from their parent. If their parent `match` is `null`, then their match will also be `null`. This means that a) any child routes/links will have to be absolute because there is no parent to resolve with and b) a pathless route whose parent `match` can be `null` will need to use the `children` prop to render.
4 changes: 1 addition & 3 deletions packages/react-router/modules/Route.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ class Route extends React.Component {
const { route } = router;
const pathname = (location || route.location).pathname;

return path
? matchPath(pathname, { path, strict, exact, sensitive })
: route.match;
return matchPath(pathname, { path, strict, exact, sensitive }, route.match);
}

componentWillMount() {
Expand Down
8 changes: 5 additions & 3 deletions packages/react-router/modules/Switch.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ class Switch extends React.Component {
const path = pathProp || from;

child = element;
match = path
? matchPath(location.pathname, { path, exact, strict, sensitive })
: route.match;
match = matchPath(
location.pathname,
{ path, exact, strict, sensitive },
route.match
);
}
});

Expand Down
47 changes: 47 additions & 0 deletions packages/react-router/modules/__tests__/Route-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from "react";
import ReactDOM from "react-dom";
import PropTypes from "prop-types";
import { createMemoryHistory } from "history";
import MemoryRouter from "../MemoryRouter";
import Router from "../Router";
Expand Down Expand Up @@ -388,3 +389,49 @@ describe("A <Route location>", () => {
});
});
});

describe("A pathless <Route>", () => {
let rootContext;
const ContextChecker = (props, context) => {
rootContext = context;
return null;
};

ContextChecker.contextTypes = {
router: PropTypes.object
};

afterEach(() => {
rootContext = undefined;
});

it("inherits its parent match", () => {
const node = document.createElement("div");
ReactDOM.render(
<MemoryRouter initialEntries={["/somepath"]}>
<Route component={ContextChecker} />
</MemoryRouter>,
node
);

const { match } = rootContext.router.route;
expect(match.path).toBe("/");
expect(match.url).toBe("/");
expect(match.isExact).toBe(false);
expect(match.params).toEqual({});
});

it("does not render when parent match is null", () => {
const node = document.createElement("div");
ReactDOM.render(
<MemoryRouter initialEntries={["/somepath"]}>
<Route
path="/no-match"
children={() => <Route component={ContextChecker} />}
/>
</MemoryRouter>,
node
);
expect(rootContext).toBe(undefined);
});
});
23 changes: 15 additions & 8 deletions packages/react-router/modules/__tests__/matchPath-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,21 @@ describe("matchPath", () => {
});

describe("with no path", () => {
it("matches the root URL", () => {
const match = matchPath("/test-location/7", {});
expect(match).toMatchObject({
url: "/",
path: "/",
params: {},
isExact: false
});
it("returns parent match", () => {
const parentMatch = {
url: "/test-location/7",
path: "/test-location/:number",
params: { number: 7 },
isExact: true
};
const match = matchPath("/test-location/7", {}, parentMatch);
expect(match).toBe(parentMatch);
});

it("returns null when parent match is null", () => {
const pathname = "/some/path";
const match = matchPath(pathname, {}, null);
expect(match).toBe(null);
});
});

Expand Down
29 changes: 29 additions & 0 deletions packages/react-router/modules/__tests__/withRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,35 @@ describe("withRouter", () => {
);
});

it("works when parent match is null", () => {
let injectedProps;
let parentMatch;

const PropChecker = props => {
injectedProps = props;
return null;
};

const WrappedPropChecker = withRouter(PropChecker);

const node = document.createElement("div");
ReactDOM.render(
<MemoryRouter initialEntries={["/somepath"]}>
<Route
path="/no-match"
children={({ match }) => {
parentMatch = match;
return <WrappedPropChecker />;
}}
/>
</MemoryRouter>,
node
);

expect(parentMatch).toBe(null);
expect(injectedProps.match).toBe(null);
});

describe("inside a <StaticRouter>", () => {
it("provides the staticContext prop", () => {
const PropsChecker = withRouter(props => {
Expand Down
12 changes: 5 additions & 7 deletions packages/react-router/modules/matchPath.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ const compilePath = (pattern, options) => {
/**
* Public API for matching a URL pathname to a path pattern.
*/
const matchPath = (pathname, options = {}) => {
const matchPath = (pathname, options = {}, parent) => {
if (typeof options === "string") options = { path: options };

const {
path = "/",
exact = false,
strict = false,
sensitive = false
} = options;
const { path, exact = false, strict = false, sensitive = false } = options;

if (path == null) return parent;

const { re, keys } = compilePath(path, { end: exact, strict, sensitive });
const match = re.exec(pathname);

Expand Down
2 changes: 1 addition & 1 deletion packages/react-router/modules/withRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const withRouter = Component => {
const { wrappedComponentRef, ...remainingProps } = props;
return (
<Route
render={routeComponentProps => (
children={routeComponentProps => (
<Component
{...remainingProps}
{...routeComponentProps}
Expand Down