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

add support for an array of paths in <Route> and matchPath #5889

Merged
merged 1 commit into from
Oct 15, 2018

Conversation

baronswindle
Copy link
Contributor

@baronswindle baronswindle commented Jan 20, 2018

I piggybacked off of the tests written for matchPath() by @Shervanator at #5393, and I'm tackling the issue described by @mjackson at #5866.

  • Added support for arrays of paths to <Route> and matchPath()
  • Added a test case to demonstrate that match.path is a string
  • Added a test case to demonstrate that the component doesn't remount on switches between two matching paths

I'm happy to add docs and examples if everybody is satisfied with the behavior. Let me know if you think any changes are in order as well.

@baronswindle baronswindle force-pushed the path-array branch 3 times, most recently from 5654844 to 6bc2f8a Compare January 20, 2018 19:41
@timdorr timdorr force-pushed the master branch 2 times, most recently from fb3a586 to f9481bb Compare January 26, 2018 13:04
@amadeus

This comment has been minimized.

@timdorr timdorr force-pushed the master branch 4 times, most recently from 8f33936 to 1fb8696 Compare April 26, 2018 20:30
@itsdouges

This comment has been minimized.

@MQuy

This comment has been minimized.

@cubeghost
Copy link

Would also love to see this merged, definitely willing to help out if it needs any more work.

@oyeanuj

This comment has been minimized.

@danvc
Copy link

danvc commented Jul 28, 2018

Why wait for #5866 when this PR is already working?

@baronswindle
Copy link
Contributor Author

@mjackson @pshrmn I've rebased this onto the tip of master and added some documentation. Let me know if you need anything else from me to merge this.

Copy link
Member

@mjackson mjackson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks @baronswindle! And thank you for rebasing on top of all the changes from the past few weeks 😅

@mjackson mjackson merged commit a4d0064 into remix-run:master Oct 15, 2018
@mjackson
Copy link
Member

Just FYI everyone, this was just released in version 4.4.0-beta.4. You can try it out now by installing from the next tag:

npm install react-router-dom@next react-router-native@next

@baronswindle baronswindle deleted the path-array branch October 24, 2018 04:13
@kevich
Copy link

kevich commented Oct 25, 2018

Hey! That looks nice, but what about support of RegExp in paths?! react-route is still not fully same as it is in https://github.com/pillarjs/path-to-regexp, which supports RegExp

@timdorr
Copy link
Member

timdorr commented Oct 25, 2018

@kevich #6418 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.