-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Feature Request: Path should treat / and // as the same thing #4445
Comments
Hmm, ya, we should fix this. Good catch :) I don't think the fix is to treat There are a couple of things we could do here:
Related: I think the relevance of |
From my perspective, I think that the second solution is one that is easier to understand. It might be confusing that However, I agree with you that this is much smaller issue if relative paths are handled gracefully. |
Related: #4459 |
Think I've come across the same thing? |
@timdorr thanks! |
Thanks for the quick fix. Very eager for this fix to be released. Preventing SSR from working for me after spending time migrating lots of code from alpha releases. |
Ran into the same issue. Snippet of component code... <ul>
<li><Link to="/">Home</Link></li>
<li><Link to="/page/about">About</Link></li>
<li><Link to="/page/contact">Contact</Link></li>
</ul>
<hr />
<Route exact path="/" component={Home} />
<Route path="/page/:name" component={Page} />`` And doing SSR via Koa: const route = {};
const html = ReactDOMServer.renderToString(
<StaticRouter location={ctx.request.url} context={route}>
<App />
</StaticRouter>,
); On the client, the 'About' link is Of course, Currently all SSR routes are broken. |
What's the ETA for the release? Thanks in advance. |
This issue concerns manually building URLs by concatenating This issue is not related to the Please only comment about the former. The latter has been fixed and should be released soon 😄 |
Sorry @pshrmn, that wasn't clear. On first glance, the OP's syntax looked near identical to my own, so I thought this was probably the same underlying thing. Do you have any ETA for the release? |
There are no ETAs. This is beta software, so you use at your own risk. Sorry to sound harsh, but you definitely should not be using this code in production and can't rely on fixes being released in a timely manner. But that being said, I would expect one by the end of the month for sure. |
@timdorr, then might I respectfully suggest you make that clear on https://reacttraining.com/react-router or the v4 README? When I ask for ETAs, it's not to place demands on your time or to suggest you're beholden to anyone. It's because I'm excited about the release, and this is a very small detail that is preventing a major part of the framework's promised benefit. The 'beta' warning in this issue is the first I've seen. It's not mentioned in the readme. The training site has a ton of (very useful) syntax, and the API seems complete. There's even a video showing it working in practice. That's far more than most projects that limp by in 'beta' for years, and accrue plenty of production users. Node itself was pre-1.0 for 6 years. I'm not sure the casual 'beta' tag is enough of a deterrent in the JS ecosystem. For my use case personally, I'm building a starter kit that puts v4 front and centre. On the react-hot-loader starter kit page, there are multiple projects listed that feature react-router 4... clearly, I wasn't alone in thinking this was ready for the wild. If v4 isn't ready, I'll obviously need to re-write. Seeing a warning not to use it would have saved me a few days. I certainly don't wish to dictate your release schedule, and I appreciate all of your hard work on this awesome package. But if it's already fixed, is there any way you could bump the patch to a new release and help out me and @carsonperrotti and others for whom this simple fix (that has already been merged!) is actually a pretty major roadblock to SSR? |
In the meantime, a crutch fix can be done with a regex replacement: serverSideHTML.replace(/<a href="\/\//g, '<a href="/') That should make both your server-side HTML and client-side HTML match but note that you will still get the warning for checksum mismatch. Looking forward to the release! :) |
that's actually almost exactly what I wound up doing - good tip, @verekia! |
@leebenson Thanks for the input. I'm the one who cuts releases from the v4 branch, so you can blame me for the slow release. We're keeping busy and pushing steadily forward. Thanks for your patience :) |
OK, 4.0.0-beta.6 is released.
Clearly, you never used Gmail ;) |
Thank you very much @mjackson! beta.6 indeed fixed it! :) |
Thanks @mjackson! |
That's exactly my point. 'Beta' doesn't deter usage. Half of my stack is alpha/beta/pre-1.0. What's ever really production-ready in the JS world? If the official suggestion is for users to consider this dev-only, I'd definitely make that clearer. But so far, I have to say this was the only real issue I came across and that's been released- so thanks 😄 |
@leebenson You're welcome :) Side note: your life as a cutting-edge software dev will be greatly simplified if you develop strategies for using not-yet-released work so you can move on with your development w/out having to bug the maintainers to cut a release. Yes, I know it's more convenient to just |
@mjackson - that's definitely good advice. Sorry if I was a PITA on this occasion, but I do appreciate the fast release. You've done an awesome job with v4. |
seems like this should be closed? please reopen if I'm wrong. |
React Router 4.0.0-beta.4
I ran into this today, and it was quite confusing.
Here is my app:
In this case, the path for the
Account
component will only match when I've come from a place wherematch.url
is not/
. Whenmatch.url
is/
then that route path is//account
and it doesn't match. For other paths, I'd be able to have a convention, where I never end my route paths with/
, for instance ifmatch.url
is/user
thenAccount
path is/user/account
which will match.The text was updated successfully, but these errors were encountered: