Add support for partial <Route /> paths #6808
Conversation
- relative paths starting with ./ like './foo' are now resolved as 'foo' - relative paths starting with .. throw an error - <Route /> and <Switch /> take care of parent matches (fixes failing tests)
| // but without escaping of slashes and dots | ||
| const escapedPath = | ||
| path && path.replace(/([.+*?=^!:${}()[\]|/\\])/g, "\\$1"); | ||
| path && path.replace(/([+*?=^!:${}()[\]|\\])/g, "\\$1"); |
There was a problem hiding this comment.
This works because responding to the regex pattern below, slashes don't need to be escaped and dots need to be wrapped by parentheses to be recognized as placeholders in the path. Given that, that we escape parentheses, there is no case that dots could be ever recognized as placeholders:
/(?:\\:(\\w+)(?:\\(((?:\\\\.|[^\\\\()])+)\\))?|\\(((?:\\\\.|[^\\\\()])+)\\))([+*?])?/
(see: https://github.com/pillarjs/path-to-regexp/blob/d1ec03afb6eefe7ad19e4d990a428e6b545fb3a6/index.js#L29)
|
@StringEpsilon, @timdorr It would be great if you could approve this pull request before it is outdated again. There are no breaking changes that break the backward compatibility. |
instead of complete parent match, just use the url of the parent match
missed to change matchRoutes() after change in previous commit
|
Ping! So the last reaction on this pull request was now nearly two months ago and there was never a real discussion about adding it or not. So how's your latest opinion about it, @StringEpsilon and @timdorr? This PR does not change the api a lot, does not create any backwards compatibility issues and does not add any new functions or something like this. I mean, it just adds a feature many users waiting for years. |
|
Won't this break existing code where people are already doing the nesting manually using the parent route's |
|
@mjackson No, because it also handles absolute paths – and that's what you achieve when you build a route with |
* explain same component in multiple routes wanted to clarify the behavior seen here: https://codesandbox.io/s/sleepy-hopper-bgntr * Update packages/react-router/docs/api/Route.md Co-Authored-By: Tim Dorr <[email protected]>
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
@timdorr Could you please remove the |
|
I'm going to look into an exclusion filter so stalebot will ignore things we mark as not stale explicitly. |
|
@timdorr thank you. |
MeiKatz
force-pushed
the
support-for-relative-paths
branch
2 times, most recently
from
712fd7c to
89d5aad
Compare
|
I adjusted my pull request to the changes that happened in the meantime so it is ready for 2020, RRv6 and hopefully a merge in foreseeable time. |
|
@timdorr could you please check what is going on with Travis? There seems to be a test that is only queued, but if I take a look on the Travis site all checks are finished and have passed. Most likely a sync problem between GitHub and Travis. |
|
I just tried restarting it. But as long as one of them works, that's OK. |
|
I'm hesitant to merge this into v5 because, although it works, we cannot also introduce support for relative links w/out introducing breaking changes. So we'll have relative routes, but not relative links. I'm kinda on the fence though because it should help smooth the upgrade path to v6 if people can replace all their |
|
Thanks for everybody's work on this. I'm closing issues that are old, if they are still problems please open a new issue and reference this one. Thanks again! |
9 participants
Actually this is nearly the same push request as #6618, that I f**ked up while updating with the changes that occurred in the time in between. So it know also supports arrays for
pathin<Route />. Hopefully this is now merged and we don't have to wait for this feature until RR v6.