[v6] Add location prop to Routes and RouteOptions to useRoutes #7571
Conversation
|
@smvv Thank you :) btw: are you a RR maintainer? |
|
no, just a user that would like this feature as well! |
|
I am just surprised that you could edit my pull request. |
|
Michael and Ryan are pretty focused on Remix at the moment, so work on React Router has been largely paused for the past 2 months. I've been marking PRs as approved where I can, but I don't feel comfortable merging things in because I both don't know what the overall vision for v6 is, nor if M+R have some Router work progressing alongside Remix that will get merged in later. That's not to say this won't get merged in, just that I don't have a heckin' clue as to when that might happen. Folks are busy trying to get businesses off the ground (I am too!), so it might be a bit of time. It'll happen eventually, though. |
|
Thanks for the perspective @timdorr! Don’t mean to make you feel under pressure, we all appreciate your hard work on RR. It’s unfortunate progress on v6 has been a bit sluggish lately. My team are really excited for the potential v6 has but it’s still missing some key components, this being one of them. Is there anything us in the community do to help move these issues along? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
|
||
| let matches = React.useMemo( | ||
| () => matchRoutes(routes, usedLocation, basename), | ||
| [location, routes, basename] |
There was a problem hiding this comment.
useMemo dependencies weren't updated here with usedLocation
|
Very sorry, I didn't actually get to this PR before merging a similar change in #7937. If you want to rebase with the current dev branch I'm happy to take another look given there are a few additional API changes in this PR we might want to consider. |
|
@chaance To be honest: I am very disappointed about the way thing are going right now. Yes, it's great that things are starting again and ReactRouter v6 seems to point to an end and might be out of beta in the near future. But there are people like me who wanted to contribute changes and waited and waited and waited in the hope that they got merged. And now our pull requests are ignored and somebody else adds them on their own. It's kind of dishonoring the work of them. Sorry, but I am not really pleased by the things that happening right now. |
|
As someone who has also been following this issue/PR for a long time, I totally understand your frustration @MeiKatz - I would be frustrated too. But in this case, it seems like an honest mistake with no ill intent. I assume since the other PR was open more recently, it surfaced higher in the review queue. I do think you and the other contribs should receive collaborative credit for this solution, though! Appreciate everyone's work in getting this fixed, I know my team appreciates it! |
|
@MeiKatz I hear you, and I apologize and assure there was no ill-intent. Last week was my first on the job and we wanted to get lots of important bug fixes out since folks have been waiting for so long on some of these. I moved a bit too fast in this case. 🙇♂️ That said, the PR we merged ended up being reverted anyway (which I'm sure may feel just as frustrating!). @mjackson started a new conversation on solving animations more directly in v6 that would eliminate the need for a |
|
I'm going to close this one out, not because I don't think this PR is good, but because there's been a lot of divergence and the revert on If the discussion does reveal a strong use case for including the |
|
@timdorr I have shared my concerns in #7946 (comment). |
I cleaned the commits of #7298 to just these two. This should be easier to merge.
Fixes #7117 and #7297.
@MeiKatz @timdorr @mjackson @ryanflorence