Accept array of strings in route path

[Shervanator]

The path-to-regexp library supports an array of strings as a pattern, however we are getting PropTypes warnings about this. This PR enables support for this.

It seems like we do not touch the path and just pass it on to the path-to-regexp library directly.

See these previous PR's and Issues for more context:
#4159
#4551

[timdorr]

Like I said in #4159, it is not as simple as that. While we do pass the path prop directly into path-to-regexp, we also use it to determine the URL for the match.

This would only be acceptable if there was a failing test added on the match's URL and a fix implemented for that failure.

[Shervanator]

@timdorr ah I missed that stuff on line 48.

We have added some test cases now for the path as array case, however to make the tests pass we had to remove the path === '/' that happened on line 48.
We don't quite understand why that check was happening in the first place, from our testing

var pathToRegexp = require("path-to-regexp")
const re = pathToRegexp(['/somewhere', ''], { end: false })
console.log(re.exec("/elsewhere/else"))

// => [""]

so in this case url is empty "" but there was a match, so that must mean that the path was either "/" or "", which feels safe to set the url to "/", but maybe you have more insight?

thanks!

[fraserxu]

Any update on this? Would love to see this get merged. 🙏

[marcdavi-es]

@timdorr if there's more work to do for this, happy to chip in.

[pshrmn]

The reason that this isn't currently done has more to do with there not being a compelling reason than with difficulty in implementing it (since there really isn't any), so any work required to get this merged is in convincing Ryan/Michael that

<Route path={[ '/one', '/two' ]} component={Thing} />

is significantly better than

<Route path='/one' component={Thing} />
<Route path='/two' component={Thing} />

[Shervanator]

Thanks @pshrmn okay I will explain why we need this feature (it will fix an issue we currently have on one of our sites in prod):

So it isn't that :

<Route path={[ '/one', '/two' ]} component={Thing} />

Is any better than:

<Route path='/one' component={Thing} />
<Route path='/two' component={Thing} />

It's that the first example will not cause a Unmount and then a Mount of the Thing component when the route changes between /one and /two, whereas the second example would.

This is an issue for us because we have some slightly complex route changes that will change the url but not the content on the page, and we are currently using the componentWillMount to fire certain events that should only happen once, however at the moment because the component gets unmounted and then mounted again that event is being fired twice.

From what I've seen in the issues linked above, it looks like others also have noticed this and would benefit from this simple change!

Happy to help in anyway to continue this discussion :)

cc @timdorr

[pshrmn]

Inside of a <Switch> (and I personally think that routes should almost always be defined inside of switches), the <Route> would not be re-mounted. https://codesandbox.io/s/RP2LXkkV (you'll need to open the console).

[Shervanator]

Thanks for the info and great example @pshrmn

So i've looked at our code and it was wrapped by the <Switch> component, we are using the react-router-config package to handle our routes.

If you take a look at the main function in that package: renderRoutes.js
It puts all the routes in a <Switch> component, which is what we want, but when I was doing some debugging it seemed like it was still unmounting and remounting our components.

We traced this down to the following: the renderRoutes function sets a key on each of the <Route> components it creates renderRoutes.js#L8. It needs to do this I guess as its rendering a list of component. However when the key is set the unmounting and mounting behaviour is present.

I've modified your example to show this in a small test case:
https://codesandbox.io/s/76A33Vgpy

Notice how the <ThingInASwitchWithKey> always mounts again, whereas the <ThingInASwitch> updates on route change.

Im not sure why this is happening, but if we can figure this out then maybe this PR is not needed to do what we want :)

Thanks again!

[marcdavi-es]

...but it is needed to cover cases where <Route>s are not inside a <Switch>

The docs expressly encourage composing routes in a bunch of different ways...

This is by design, allowing us to compose <Route>s into our apps in many ways, like sidebars and breadcrumbs, bootstrap tabs, etc.
Occasionally, however, we want to pick only one <Route> to render ... do it with Switch:

It feels a bit meh that this

<Route path={[ '/one', '/two' ]} component={Thing} />

and this

<Route path='/one' component={Thing} />
<Route path='/two' component={Thing} />

happen to be equal if your application fits Switch-style routing, but unequal if it fits composed routing.

Isn't it much cleaner to be able to say <Route> accepts any valid URL path that path-to-regexp understands.

It's just one merge away... 🤞

---

UPDATE - just cc'ing @timdorr / @pshrmn for thoughts?

[Shervanator]

hey @timdorr and @pshrmn any updates :)

[marcdavi-es]

@timdorr and @pshrmn any thoughts on my thoughts? :)

[marcdavi-es]

@timdorr / @pshrmn should we abandon hope?

[pshrmn]

This isn't my call to make. Ryan/Michael are the ones that will have to make this decision (but please don't ping them, they can see all of the PRs here and will get to this eventually). Based on past rejections of the same request, I would say that this is unlikely to be added, but I won't dismiss it altogether.

[mjackson]

For most people who have this problem (want to render the same component at 2 different paths) I'd say they should just map over the array and render 2 <Route>s (same as @pshrmn recommended).

But if you want to avoid re-mounting the component when the location changes, it's a little more tricky because you'll need to re-use the same <Route> component. The only way I can think of to do this currently would be to use a route that matches either path, e.g. <Route path="/(one|two)">.

Of course, at some point in your logic you're going to have to differentiate between the paths and do something like:

if (location.pathname === '/one') {
  // do something for /one...
} else if (location.pathname === '/two') {
  // do something for /two...
}

Would that work for your use case, @Shervanator?

[MrJadaml]

Very much looking forward to this being merged in.

[misterfresh]

Also would want this merged.

[marcdavi-es]

Me too. @Shervanator are you still with us? :)

[fraserxu]

I see the way to work around the problem, but there's seem no clear reason to not merge it 🤔

[vuhrmeister]

I also like to see it merged.

But if not, the docs should be adapted: https://reacttraining.com/react-router/web/api/Route/path-string

Any valid URL path that path-to-regexp understands.

In fact, that's currently not true! So at this point there should be a hint that only strings are accepted.

[benviau]

I have the same use case described above as well as in 4551. That is, multiple exact routes should render the same component, and I do NOT want the component to re-mount when the location changes between acceptable routes. As vuhrmeister mentioned, the docs explicitly state that I can use any path that path-to-regexp understands - which as it currently stands is incorrect. The implication is that I've spent too much time trying to figure out what I'm doing wrong, when in fact the documentation itself is wrong. I'd really like to see this PR merged, but at the very least, I'd much appreciate if the docs could be adjusted to explain that only strings are permitted.

[lamuertepeluda]

I just want to add that having to re-mount the component is really ugly, for instance, if you are using route transitions and can also have a lot of other side effects... I'd really love to see this merged in.
So far I'm just ignoring the annoying warning.
Any chances to get this merged in the next v4.x release?

[tmbdrogba]

What remains to be done to see this PR merged ?

[mjackson]

I'm not totally opposed to the idea of <Route path> accepting an array, but the way things currently work this change breaks code that builds relative <Route>s based on the match.path.

For example, if you wanted to build a relative <Route> that built upon its parent's path, we would normally recommend you do something like this:

<Route path="/users/:id" render={match => (
  <Route path={`${match.path}/profile`} ... />
)}>

This would create 2 routes: one at /users/:id and another at /users/:id/profile. The inner <Route> uses its parent's path to build its own. However, if we allow <Route path> to be an array, this code breaks:

<Route path={[ '/one/:id', '/two/:id' ]} render={match => (
  <Route path={`${match.path}/profile`} ... /> // boom! :/
)}>

So unfortunately I can't just merge this change as it stands. I'll update the docs so people know that we only accept strings there, but I'm open to ideas about how this could be improved in the future.

Thanks everyone for the discussion!

[tmbdrogba]

@mjackson With your code, you can't do the same thing for two differents path : /foo and /test.

If you use two <Route .../> with those two urls and the same component, the component will unmount and remount, and of course it will loose its state and everything else...

[lamuertepeluda]

@mjackson I tried to reproduce your example... The problem is that, using Array, the React DOM tree is rendered like this (from React dev tools)

<Route path={[ '/users/:id', '/people/:id' ]} render=render()>
   <Route path='/users/:id,/people/:id'/profile' render=render()/></Route> 
</Route>

If the props returned from match where coherent, that is Array --> Array and String --> String instead of having

Array --> Array.join(',') and String --> String

then it could work

See the code here https://stackblitz.com/edit/react-j6qceq?embed=1&file=BasicExample.js

[timdorr]

@tmbdrogba

If you use two <Route .../> with those two urls and the same component, the component will unmount and remount, and of course it will loose its state and everything else...

You can avoid this with the key prop. As long as they are the same component type, that will trick it into re-using the same component.

[tmbdrogba]

@timdorr Can you provide me an example ?

I've tested

{['/foo', '/test'].map(url => <Route key={url} path={url} exact component={MyComponent}/>)}

and MyComponent is remount everytime I switch betwenn /foo and /test

[mjackson]

I think you're right, @lamuertepeluda. I've opened #5866 to track this work. As long as we can make sure that match.path is still a string, we can add this.

[yuri-sakharov]

I just tried path={['a','b','c']} and it's works fine with the exception of

Warning: Failed prop type: Invalid prop `path` of type `array` supplied to `Route`, expected `string`.
    in Route

also works option path="/(a|b|c)" without warning

[richardhsueh]

<Route
    path={[`${match.url}/service`, `${match.url}/news`]}
    render={props => <ServicePage mode={mode} {...props} theme={this.checkTheme()} />}
/>

This line of code work fine, but it still has the following warning in the console

Warning: Failed prop type: Invalid prop path of type array supplied to Route, expected string

I am using version 4.3.1.