Upgraded react-router-dom from 4.2.2 → 4.3.1
Node v8.11.2
npm v5.6.0
With the following dependencies:
"babel-core": "^6.26.3",
"babel-loader": "^7.1.4",
"babel-preset-env": "^1.7.0",
"babel-preset-react": "^6.24.1",
"react": "^16.4.0",
"react-dom": "^16.4.0",
"react-router-dom": "^4.3.1",
"webpack": "^4.10.2",
"webpack-cli": "^3.0.1"
After upgrading, the 'active'
class is no longer applied to the <NavLink/>
for active pages. I've tried explicitly adding the activeClassName
prop to the component and it still doesn't add the class the the element.
I haven't been able to create a minimal verifiable example. Has anyone else seen this? Everything else works perfectly fine, builds like normal, but the active class isn't added on the updated version.
It works as expected with v4.2.2 but not with v4.3.1 (or v4.3.0)
Props being passed to NavLink:
activeClassName: "active"
ariaCurrent: "true"
children: Array[2]
className: "img-link"
exact: true
to: "/"
JSX and HTML:
<NavLink exact={true} to="/" className="img-link">
↓
<a class="img-link active" aria-current="true" href="#/">
Props being passed to NavLink:
activeClassName: "active"
aria-current: "page"
children: Array[2]
className: "img-link"
exact: true
to: "/"
JSX and HTML:
<NavLink exact={true} to="/" className="img-link">
↓
<a class="img-link" href="#/">
Can you put something together on codesandbox.io? This needs a repro to be diagnosed.
I wasn't able to reproduce it on codesandbox or a barebones app I created. Both worked fine using the same dependency versions...
I'm encountering the same issue w/ 4.3.1, but likewise have not been able to reproduce it in a simple app - will be sure to update here if I can get a basic reproduction.
I'm also seeing this after upgrading from 4.2.2
to 4.3.1
Workaround: pass a function to the isActive
prop to do the check manually, eg:
const isActive = (path, match, location) => !!(match || path === location.pathname);
<NavLink to="/my/path"
className="nav-link"
activeClassName="nav-link--active"
isActive={isActive.bind(this, '/my/path')}>
Click Me
</NavLink>
Can anyone create a reproduction of this on codesandbox.io or just point to a repo with the issue? There really needs to be more code to look at to diagnose what's going on. My best guess is the new path escaping, but that's purely a guess at this point.
@timdorr I'm running into the issue as well. By using the react-devtools extension, I could see that the Route returned by NavLink has an extra backslash at the beginning of the path.
You can test it in your browser console
````js
// https://github.com/ReactTraining/react-router/blob/ae35a273bdb0fdd2fec32893ed1a62630da8bf3b/packages/react-router-dom/modules/NavLink.js#L25
'/foo'.replace(/([.+*?=^!:${}()[]|/\])/g, "\$1")
// using OSX returns "\/foo" in Chrome or "\/foo" in FireFox + Safari idk about windows/ie
````
We've had this issue reported by someone using tabler-react.
There is reproducing codesandbox here. If we move the SiteWrapper
component into the Example
and Dashboard
components, rather than wrapping Switch
, the problem goes away.
I just took a quick look at those two sandboxes and the broken one works when I refresh it, which is weird but I don't know if it has anything to do with the issue. If I take out the tabler-react
components and just render <NavLink>
s, it works, too.
If someone can put together a barebones (just React Router and host components) sandbox that duplicates the issue, it would be appreciated.
Same issue here with 4.3.1, I have an ssr app and the class is being added from the code rendered on the server but not when changing views on the client.
I rolled back to 4.2.2 and all is now good.
I tried to debug and match was always null in NavLink
at this location.
I put something together here that doesn't recreate the issue: https://codesandbox.io/s/0p78xv2v5p In NavLink.js, I put both the 4.2 and 4.3 versions of NavLink in there. Both work just fine...
I'm now wondering if it has to do with #5964.
I think its related to https://github.com/ReactTraining/react-router/issues/6193.
So when I was debugging and it is the first time I've really looked at the code, I stripped it back to one route on "/". It initially found a match but then it seemed to render another route for the children of the component defined in the route with a weird \/
route for which it could not find a match and so match was null in NavLink
and hence no activeClass.
@dagda1 Can you throw that stripped back code into a sandbox? Or is this only an issue for you with SSR?
@pshrmn I'm pretty sure this is only ssr related
@pshrmn @dagda1 the issue is not SSR related for us
@jonthomp I just took a closer look at tabler-react
and the <Nav.Item>
is a pure component, so when the location changes, the <NavLink>
s aren't being re-rendered (the blocked updates problem). That will go away when RR adopts the new context, but isn't the same issue as other people are experiencing.
To everyone else in this thread: can someone _please_ put together a small repo or sandbox?
I created a small repo that reproduces the bug: https://github.com/dotspencer/react-router-bug
One curious thing I discovered is that the problem goes away when I move the express package from "dependencies"
to "devDependencies"
in package.json
.
@dotspencer React Router's path-to-regexp
dependency is v1.7.0
while Express's is v0.1.7
. During installation, the path-to-regexp
dependency for Express is installed before the one for React Router, so you end up with a node_modules
that looks like this:
express
[email protected]
react-router
node_modules
[email protected]
Your Webpack configuration is set to resolve directly from the root node_modules
, so when you build your project you are building it with the old path-to-regexp
. The solution, based on this comment is to use a generalized node_modules
reference in your Webpack config.
resolve: {
modules: [
path.resolve('./src'),
- path.resolve('./node_modules'),
+ 'node_modules',
],
},
If anyone else is running into this issue and uses Express + Webpack, can you please verify if this fixes your problem. (cc @dagda1)
Absolutely worked for me also. I would never have suspected that.
I have a mono repo. Still not entirely sure how this fixes it for the monorepo.
I'm not sure if this is related to this issue. But when I use relative path, the activeClassName
is not applied.
For example,
<NavLink
to="./test"
activeClassName="is-active"
exact={true}
>
Test
</NavLink>
Active class is not applied.
<NavLink
to="/test"
activeClassName="is-active"
exact={true}
>
Test
</NavLink>
Active class is applied.
The url routes to the correct path but no active class.
I ran into the same issue but could not use the generalized 'node_modules'
workaround. Instead, I hard-coded path-to-regexp
to always resolve to the newer version via alias.
resolve: {
alias: {
'path-to-regexp': path.resolve(
__dirname, 'node_modules', 'react-router', 'node_modules', 'path-to-regexp'
)
},
modules: {...}
}
I'm not sure if this is related to this issue. But when I use relative path, the
activeClassName
is not applied.For example,
<NavLink to="./test" activeClassName="is-active" exact={true} > Test </NavLink>
Active class is not applied.
<NavLink to="/test" activeClassName="is-active" exact={true} > Test </NavLink>
Active class is applied.
The url routes to the correct path but no active class.
I'm not sure if this is related to this issue. But when I use relative path, the
activeClassName
is not applied.For example,
<NavLink to="./test" activeClassName="is-active" exact={true} > Test </NavLink>
Active class is not applied.
<NavLink to="/test" activeClassName="is-active" exact={true} > Test </NavLink>
Active class is applied.
The url routes to the correct path but no active class.
This worked for me,
<NavLink to="completed">Completed</NavLink>
But if I put the absolute path <NavLink to="/completed">Completed</NavLink>
it worked for me.
Escaping path in NavLink seems to be not needed as <NavLink to="/path"/>
seems to be rendering <Route path="\/path" />
because of it
reverting this seems to fix it for me
https://github.com/ReactTraining/react-router/commit/2d801516127273752fca93e20f2667de8f63b875
@dotspencer React Router's
path-to-regexp
dependency isv1.7.0
while Express's isv0.1.7
. During installation, thepath-to-regexp
dependency for Express is installed before the one for React Router, so you end up with anode_modules
that looks like this:express [email protected] react-router node_modules [email protected]
Your Webpack configuration is set to resolve directly from the root
node_modules
, so when you build your project you are building it with the oldpath-to-regexp
. The solution, based on this comment is to use a generalizednode_modules
reference in your Webpack config.resolve: { modules: [ path.resolve('./src'), - path.resolve('./node_modules'), + 'node_modules', ], },
If anyone else is running into this issue and uses Express + Webpack, can you please verify if this fixes your problem. (cc @dagda1)
Hi @pshrmn , is there a way to get this worked if we not using webpack.config but instead using create-react-app to setup the react app. is ejecting the app only way to this fixed in this case ?
Any news on this? I'm using a mono-repo with Express and Create-react-app so the only solution so far seems to be to eject, which I'd rather avoid (In that case it seems simpler to use the function in NavLink to do the comparison manually).
@gothbarbie it seams that this issue is fixed with 4.4.0-beta builds (I have switched to 4.4.0-beta.6)
Until 4.4 is stable, I'm using this workaround... It's not ideal since I have to specify the path twice when calling the component.
const matchWorkaround = (pathname) => (isMatch, location) => isMatch || location.pathname.startsWith(pathname);
const AccountMenu: React.SFC<IProps> = ({ accountNumber, isAdvisor }: IProps) => (
<div style={styles.wrapper}>
<NavLink
activeStyle={styles.active}
style={styles.menuItem}
to={`/banklink/accounts/${accountNumber}/current-allocation`}
isActive={matchWorkaround(`/banklink/accounts/${accountNumber}/current-allocation`)}
>
Formuesammensætning
</NavLink>
<NavLink
activeStyle={styles.active}
style={styles.menuItem}
to={`/banklink/accounts/${accountNumber}/deposit-allocation`}
isActive={matchWorkaround(`/banklink/accounts/${accountNumber}/deposit-allocation`)}
>
Fordeling af nye indskud
</NavLink>
I'm not sure if this is related to this issue. But when I use relative path, the
activeClassName
is not applied.For example,
<NavLink to="./test" activeClassName="is-active" exact={true} > Test </NavLink>
Active class is not applied.
<NavLink to="/test" activeClassName="is-active" exact={true} > Test </NavLink>
Active class is applied.
The url routes to the correct path but no active class.
@paustria were you able to solve this? the URL routes but no active class is applied...
@ameer157 I'm currently solving by not using .
or using next.js
. I did a test on version 4.4.0-beta.6
. It's still broken and it seems to be more buggy. Now, /
path always show active class name on <NavLink>
. There might be changes on how to use <NavLink>
on a new version.
Until 4.4 is stable, I'm using this workaround... It's not ideal since I have to specify the path twice when calling the component.
const matchWorkaround = (pathname) => (isMatch, location) => isMatch || location.pathname.startsWith(pathname); const AccountMenu: React.SFC<IProps> = ({ accountNumber, isAdvisor }: IProps) => ( <div style={styles.wrapper}> <NavLink activeStyle={styles.active} style={styles.menuItem} to={`/banklink/accounts/${accountNumber}/current-allocation`} isActive={matchWorkaround(`/banklink/accounts/${accountNumber}/current-allocation`)} > Formuesammensætning </NavLink> <NavLink activeStyle={styles.active} style={styles.menuItem} to={`/banklink/accounts/${accountNumber}/deposit-allocation`} isActive={matchWorkaround(`/banklink/accounts/${accountNumber}/deposit-allocation`)} > Fordeling af nye indskud </NavLink>
Hi @esbenvb,
I can't get this workaround to work for some reason, the active class is still bugged...
P.S. what is: style={styles.menuItem}
? 👍
I ran into the same issue but could not use the generalized
'node_modules'
workaround. Instead, I hard-codedpath-to-regexp
to always resolve to the newer version via alias.resolve: { alias: { 'path-to-regexp': path.resolve( __dirname, 'node_modules', 'react-router', 'node_modules', 'path-to-regexp' ) }, modules: {...} }
I thought this might be fixed in v5 but it is not. I still have to use the workaround shown above by apvilkko
@esbenvb I'm having this problem too. Haven't tried it but couldn't you have matchWorkaround
use the this.props.to
of the NavLink for matching?
Version 5.0.0 fixed this. Thanks!
I don't see how this got fixed in 5.0.0. I have links like
<Link to="page" activeClassName="active">Link</Link>
which gets resolved (correctly) to
<a href="/products/product-name-1/page">Link</a>
and when I'm on /products/product-name-1/page
, activeClassName is not added.
@wojtekmaj have you tried npm i [email protected] -S
@dagda1 I have only one copy of path-to-regexp in my repo, and it's exactly 1.7.0.
[email protected] is the latest working package in 06/09/2019. Use this package so that you can experience the behavior mentioned in the documentation.
activeClass problem is no more. :)
i had this problem on 5.0.1. i’m fairly sure it was getting clobbered by webpack-dev-server’s express reference
Passing in a callback to isActive, the match argument is returning null, location is fine. activeClass and activeStyle are not working for me.
@eulow that is the behviour I experienced, I implemented my own isActive and noticed no matter what I did a match was not found. I even ran the latest path-to-regexp locally through the node console with the same arguments and a match was found.
Are you using react-scripts or create react app?
I think the path-to-regexp
is still getting clobbered by webpack-dev-server's express reference in some circumstances.
Create react app is making react-scripts
a dependency and not a devDependency.
This only happened on a production build.
my fix was
npm i [email protected] -S
A match was then magically found.
[email protected] is the latest working package in 06/09/2019. Use this package so that you can experience the behavior mentioned in the documentation.
activeClass problem is no more. :)
Hey, I was wrong. Again I ran into the same problem in a different project. This time I found the real fix. Update your "path-to-regexp" package to latest as mentioned by @dagda1.
don’t update to the latest package. update to
npm i [email protected] -S
Please consider reopening the issue as the exact same reported issue happens when pre-rendering SPA also
Most helpful comment
@dotspencer React Router's
path-to-regexp
dependency isv1.7.0
while Express's isv0.1.7
. During installation, thepath-to-regexp
dependency for Express is installed before the one for React Router, so you end up with anode_modules
that looks like this:Your Webpack configuration is set to resolve directly from the root
node_modules
, so when you build your project you are building it with the oldpath-to-regexp
. The solution, based on this comment is to use a generalizednode_modules
reference in your Webpack config.If anyone else is running into this issue and uses Express + Webpack, can you please verify if this fixes your problem. (cc @dagda1)