Skip to content

Commit f5f2107

Browse files
committed
Remove auto-escaping from <NavLink>
Escaping should be done by the end user and the path passed directly through to the underlying <Route>. Also, update tests.
1 parent 4992522 commit f5f2107

File tree

5 files changed

+313
-261
lines changed

5 files changed

+313
-261
lines changed

packages/react-router-dom/modules/Link.js

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import React from "react";
22
import PropTypes from "prop-types";
3-
import invariant from "invariant";
43
import { createLocation } from "history";
4+
import invariant from "invariant";
5+
56
import RouterContext from "./RouterContext";
67

7-
const isModifiedEvent = event =>
8-
!!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
8+
function isModifiedEvent(event) {
9+
return !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
10+
}
911

1012
/**
1113
* The public API for rendering a history-aware <a>.
@@ -15,7 +17,7 @@ class Link extends React.Component {
1517
replace: false
1618
};
1719

18-
handleClick = (event, history) => {
20+
handleClick(event, context) {
1921
if (this.props.onClick) this.props.onClick(event);
2022

2123
if (
@@ -26,37 +28,32 @@ class Link extends React.Component {
2628
) {
2729
event.preventDefault();
2830

29-
const { replace, to } = this.props;
31+
const method = this.props.replace
32+
? context.history.replace
33+
: context.history.push;
3034

31-
if (replace) {
32-
history.replace(to);
33-
} else {
34-
history.push(to);
35-
}
35+
method(this.props.to);
3636
}
37-
};
37+
}
3838

3939
render() {
40-
const { replace, to, innerRef, ...props } = this.props; // eslint-disable-line no-unused-vars
41-
42-
invariant(to !== undefined, 'You must specify the "to" property');
40+
const { innerRef, replace, to, ...props } = this.props; // eslint-disable-line no-unused-vars
4341

4442
return (
4543
<RouterContext.Consumer>
46-
{router => {
47-
invariant(router, "You should not use <Link> outside a <Router>");
44+
{context => {
45+
invariant(context, "You should not use <Link> outside a <Router>");
4846

49-
const { history } = router;
5047
const location =
5148
typeof to === "string"
52-
? createLocation(to, null, null, history.location)
49+
? createLocation(to, null, null, context.location)
5350
: to;
51+
const href = location ? context.history.createHref(location) : "";
5452

55-
const href = history.createHref(location);
5653
return (
5754
<a
5855
{...props}
59-
onClick={event => this.handleClick(event, history)}
56+
onClick={event => this.handleClick(event, context)}
6057
href={href}
6158
ref={innerRef}
6259
/>
@@ -68,12 +65,14 @@ class Link extends React.Component {
6865
}
6966

7067
if (__DEV__) {
68+
const toType = PropTypes.oneOfType([PropTypes.string, PropTypes.object]);
69+
7170
Link.propTypes = {
71+
innerRef: PropTypes.oneOfType([PropTypes.string, PropTypes.func]),
7272
onClick: PropTypes.func,
73-
target: PropTypes.string,
7473
replace: PropTypes.bool,
75-
to: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired,
76-
innerRef: PropTypes.oneOfType([PropTypes.string, PropTypes.func])
74+
target: PropTypes.string,
75+
to: toType.isRequired
7776
};
7877
}
7978

packages/react-router-dom/modules/NavLink.js

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,46 +4,48 @@ import PropTypes from "prop-types";
44
import Route from "./Route";
55
import Link from "./Link";
66

7+
function joinClassnames(...classnames) {
8+
return classnames.filter(i => i).join(" ");
9+
}
10+
711
/**
812
* A <Link> wrapper that knows if it's "active" or not.
913
*/
1014
function NavLink({
11-
to,
12-
exact,
13-
strict,
14-
location,
15+
"aria-current": ariaCurrent,
1516
activeClassName,
16-
className,
1717
activeStyle,
18-
style,
19-
isActive: getIsActive,
20-
"aria-current": ariaCurrent,
18+
className: classNameProp,
19+
exact,
20+
isActive: isActiveProp,
21+
location,
22+
strict,
23+
style: styleProp,
24+
to,
2125
...rest
2226
}) {
23-
const path = typeof to === "object" ? to.pathname : to;
24-
25-
// Regex taken from: https://github.com/pillarjs/path-to-regexp/blob/master/index.js#L202
26-
const escapedPath = path && path.replace(/([.+*?=^!:${}()[\]|/\\])/g, "\\$1");
27-
2827
return (
2928
<Route
30-
path={escapedPath}
29+
path={typeof to === "object" ? to.pathname : to}
3130
exact={exact}
3231
strict={strict}
3332
location={location}
3433
children={({ location, match }) => {
35-
const isActive = !!(getIsActive ? getIsActive(match, location) : match);
34+
const isActive = !!(isActiveProp
35+
? isActiveProp(match, location)
36+
: match);
37+
38+
const className = isActive
39+
? joinClassnames(classNameProp, activeClassName)
40+
: classNameProp;
41+
const style = isActive ? { ...styleProp, ...activeStyle } : styleProp;
3642

3743
return (
3844
<Link
39-
to={to}
40-
className={
41-
isActive
42-
? [className, activeClassName].filter(i => i).join(" ")
43-
: className
44-
}
45-
style={isActive ? { ...style, ...activeStyle } : style}
4645
aria-current={(isActive && ariaCurrent) || null}
46+
className={className}
47+
style={style}
48+
to={to}
4749
{...rest}
4850
/>
4951
);
@@ -53,29 +55,31 @@ function NavLink({
5355
}
5456

5557
NavLink.defaultProps = {
56-
activeClassName: "active",
57-
"aria-current": "page"
58+
"aria-current": "page",
59+
activeClassName: "active"
5860
};
5961

6062
if (__DEV__) {
63+
const ariaCurrentType = PropTypes.oneOf([
64+
"page",
65+
"step",
66+
"location",
67+
"date",
68+
"time",
69+
"true"
70+
]);
71+
6172
NavLink.propTypes = {
62-
to: Link.propTypes.to,
63-
exact: PropTypes.bool,
64-
strict: PropTypes.bool,
65-
location: PropTypes.object,
73+
"aria-current": ariaCurrentType,
6674
activeClassName: PropTypes.string,
67-
className: PropTypes.string,
6875
activeStyle: PropTypes.object,
69-
style: PropTypes.object,
76+
className: PropTypes.string,
77+
exact: Route.propTypes.exact,
7078
isActive: PropTypes.func,
71-
"aria-current": PropTypes.oneOf([
72-
"page",
73-
"step",
74-
"location",
75-
"date",
76-
"time",
77-
"true"
78-
])
79+
location: PropTypes.object,
80+
strict: Route.propTypes.strict,
81+
style: PropTypes.object,
82+
to: Link.propTypes.to
7983
};
8084
}
8185

0 commit comments

Comments
 (0)