-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Bring back <Route name> #1840
Comments
One major benefit of bringing this feature back will be less upgrade pain. Throughout the multiple apps I have in production, there are hundreds of |
Just my 2 cents having been following the discussion: I agree with the reasoning behind this decision (e.g. offer a set of primitive APIs so that if you need a more advanced solution, you can implement it yourself). It would be very nice indeed though if the equivalent of a component would be implemented by the core developers and maintained as an add-on for the router. |
I see it's a tough decision to make. Dynamic route loading is a pretty valid and strong reason though. But still I'm... reluctant to visualize a large app with all urls hardcoded. Kind of decreases component reusability, right? It also decreases the readability of the paths. Would anyone want to have to look at paths like these? — render() {
let trip = this.props.trip;
let solution = this.props.solution;
return (
<Link to={`/booking/${booking.id}/${solution.getId(this.props.trip.id)}?partner=${partner.id}`} />
);
} Yes, this can be lightened up if you cache enough object lookups and function calls and maybe restructure your props a little, but still it's not really nice to look at, although the compiled url is going to be relatively simple. I propose something intermediate. render() {
let bookingParams = {
id: this.props.id,
solution_id: this.props.solution.getId(this.props.trip.id),
partner: partner.id
};
return (
<Link to="/booking/:id/:solution_id?partner" params={bookingParams} />
);
} This way, all |
I might be one of the odd ones, but I found that named routes confusing at times due to there being two ways to reference a route. In the codebase, there were some references by name, some by path, and (the worst...) some names that looked like paths but weren't. While this could be remedied by more thorough code review, it could be avoided entirely by having this functionality isolated to an addon component. Having an addon component reduces the api surface of the core library and makes the decision to use named routes an explicit choice, but would still provide a way forward for those that desire/already extensively use named routes. |
Like @danmartinez101 I found it easy to create confusingly ambiguous route names and paths. But I think it would be great to still have builtin support for named routes as long as |
pre-1.0, we were dispatching data fetching on the server using an object that keyed on the {
'home': fetchDataForHome
} In 1.0, since the suggestion seems to be moving to sticking data-loading on the Routes themselves, (doc) we've moved/are moving away from |
I also think @maspwr making a good points here. Another thing that worries me every time I do |
I'm probably missing the point. Why not just write render() {
let { id, solution, trip, partner } = this.props;
let solutionId = solution.getId(trip.id);
return (
<Link to={`/booking/${id}/${solutionId}?${partnerId}`} />
);
} ? This is shorter and more direct than both of the versions above. |
This is a great point. (And it also applies to the query: you don't want However this point is not really about named routes: it is solvable with // <a href="/foo/1%2Fbaz?q=ryan%26michael
<Link to={{
path: ['foo', '1/baz'], // every segment is escaped
query: {
search: 'ryan&michael' // every query parameter is escaped too
}
}} /> Is allowing an object export function userProfile(id, page) {
return {
path: ['users', id],
query: { page }
};
} (in userland) |
+1 on supporting objects |
Another use case when named routes can be handy, is when we build a reusable module that supposed to have several pages and links between them, but it doesn't know the base url. A module like that could provide only a subtree of router config, but the end user decides where to put that subtree in the whole router config tree. So the problem is that such module wouldn't be able to build a full path for a link to one of it's pages. But with named routes it could just specify the name. |
Yeah, and another advantage of using route names is that it made checking for active routes relatively simple. For example, if you have a parent route function isActiveRoute(activeRoutes, routeName) {
return activeRoutes.some(route => route.name === routeName);
}
if (isActiveRoute(router.getActiveRoutes(), 'users') &&
!isActiveRoute(router.getActiveRoutes(), 'specificUser') {
// ...
} The only way I've found to do this via const { location, params } = this.props;
const { pathname } = location;
if (pathname.indexOf('/fen' === 0) && // this "indexOf" check seems very brittle..
!params.id) {
// ...
} |
Please, bring it back. As extension or into the core, but you should to do this. Almost every paragraph in your migration guide starts with "As we removed naming routes, so...". Except a lot of breaking API, it became horrible. Named routes are the most important feature of any router, and every good router implement this feature.
But instead of this, I have to know how path looks like, |
You can help by taking a few hours to figure out an API for a |
For example you can take https://github.com/rcs/route-parser and write a |
Seriously, route matching is not an easy problem. I can't speak for Michael or Ryan but from what I saw, there was plenty of effort spent on fixing different bugs with it. Add to this the burden of maintaining a custom arbitrary syntax, documentation for it, and receiving “let's add this feature to matcher!” requests. One library can't do many things well. There are limited people working on it, and limited effort. The focus of React Router is on matching history changes to updating React component tree. This was the reason history was separated: until it was a separate project with its own set of tests and a public API, it was difficult to focus on making it correctly handle all the edge cases. Route matching is another problem that seems simple, but is in fact difficult to get right, and is not really related to router's main purpose: matching URLs to component trees. While traditionally it has been part of routers, I think it's not a valid argument, because traditionally routers are much simpler and route matching is all they do. React Router 1.0 API makes it easy for you to build your own thing. Go for it. Named routes are a good use case, but they're not central to what React Router does, and can live as a plugin just fine if somebody actually cares about them enough to take time to implement them instead of being concerned on Github. ;-) |
Everything everybody wants here is easily implemented in user land with your own history middleware and Link component. Let's start seeing some :) |
In my experience with an app with several dozen routes, the names and the paths were identical, except the intermediary dynamic segments were replaced with some delimiter like export default class extends React.Component {
render () {
return <Link {...this.props} to={interpolate(this.props.path, this.props.params)}/>
}
} |
I just spent an entire night migrating a fairly large app to 1.0.0-rc1. At first I missed the named routes a lot, especially since we were doing magical things with it in multiple webpack bundles, e.g. could reuse 1.0 is looking out to be a great release! Hats off to @mjackson, @ryanflorence and all contributors, you've done an amazing job! |
Well for the sake of the conversation, and for figuring out where the limitations if userland implementations here is one. It implements names and "areas" in a .NET mvc-esque fashion (more closely mirrors our backend). Names can map to more than one route, and you may say that is crazy, but it allows helpful url name patterns at the expense of some ambiguity, and that is a fine trade off for us. https://gist.github.com/jquense/109f86a6b443345bfd76 There are a few problems, the biggest is that it doesn't work with server rendering, which is fine for us we don't do that. My big question is: is there an easier/better way to extend the router context? The other issue is that I feel like I had to reach into lib and utils a lot, which makes me uneasy that I may be relying on private API's to accomplish a lot of this |
Side point to the issue of why folks are so adamant about this feature. It think its just that people now have infrastructure built on this feature. Its great that we can implement it, but I think the main reason folks choose a lib in the first place is so they don't need to own that code. I know that the cost associated with code is very evident to the maintainers here, who have really graciously turned away business to get this router out the door. That is very kind and not taken for granted! The rest of us to a lesser extent are also squaring the some of those concerns with having to backfill something that our chosen library once provided. I mention this not as a reason or argument but maybe as some context to help us all get along. The React ecosystem doesn't have a lot of robust options for routing aside from RR, so while it is the right choice for RR to refocus its API to provide a better foundation for extensions and platforms. None of those extensions/platforms exist yet, and the choice to build/maintain those new extensions is not as simple as "does RR technically allow for this extension". all that being said. this is an amazing package, and ya'll are all excellent people for providing it at cost to yourselves, for the sake of the community, so thank you, and the new API is awesome. |
+1 |
The benefit of named routes are:
|
I'm missing the route names a lot too, for all the reasons @rande lists above me. While migrating an app from 0.13 to 1.0.0-rc1, I didn't want to duplicate the paths in a separate lookup, so I've written a quick'n'dirty helper that takes a route definition, and recursively creates a lookup of all name/path combinations, so it can be used in an let lookupByName = (route, prefix = route.props.path) => {
let lookup = {};
React.Children.forEach(route.props.children, (child) =>
lookup = {...lookup, ...lookupByName(child, prefix + (prefix.slice(-1) === '/' ? '' : '/') + child.props.path)}
);
if(routes.type.displayName === 'Route' && route.props.name)
lookup[route.props.name] = prefix;
return lookup;
}
let lookup = lookupByName(<Route path="/" component={...}>...</Route>); For extra dirtyness points, add the lookup as a property to the history context so it is accessible at any callsite of pushState. |
@mjackson |
So do route creator functions, like @gaearon suggested. I think those represent the best pattern here. They are more of a documentation change and less of an API to implement, as they are pure functions that shouldn't be all that complex and require any wrapping. If you want to get fancy about it, something like route-parser would be a neat, optional addon library to reduce some boilerplate. Or maybe there could be a new project (akin to |
@rpominov 👍 :D |
This thread is tired, and I think we may have a good solution going forward in #2177. Let's follow up there. |
As an aside, for reasons unclear to me I actually had to add support for naming routes on |
Well, this is a pretty huge change blocking our adoption of react 0.14. Please deprecate in the future. |
They're back. Brand spanking new: https://www.npmjs.com/package/use-named-routes. |
library like |
I recommend not bringing in a separate URL resolution library. The idea behind use-named-routes is to leverage the new location descriptor functionality in History to add drop-in support for named routes. The idea is that it just works without any monkey-patching or hackery involved. You can seamlessly do both: <Link to="widgets">
<Link to={{ name: 'widget', params: { widgetId: 'foo' } }}>
<Link to="/widgets/foo"> And use the route names and params anywhere where you currently need a path, including e.g. router.push('widgets');
router.push({ name: 'widget', params: { widgetId: 'foo' } });
router.push('/widgets/foo'); And it all just works. |
Incredible work, man! |
I was excited to see named routes in core mentioned in the roadmap of @taion's fork blog post, if I remember correctly. Is that still intended? |
@mwhite what don't you like about the post above? #1840 (comment) |
@mwhite I don't think we plan on adding them to core. I expect that the named route support will live in an extension for the time being. |
Allow me to quote from this excellent piece:
And -- gasp -- it's possible for the same package to contain both the core submodule and a meta-module that exposes a user-friendly interface through some combination of wrappers, multiple supported calling styles, and overridable default extensions. Ergonomics are important. Users shouldn't have to import an external package to get named routes, because that's one more package to worry about and whether the extension API changes. |
That's certainly an option we're considering. However, upstream React does use the "addon package" pattern quite extensively. See https://www.npmjs.com/package/react-addons-css-transition-group and https://www.npmjs.com/package/react-addons-shallow-compare most notably. We agree that this is a feature that many users will want – otherwise I wouldn't have taken the time to code it up in the first place (as I don't use it) – and that is a factor we will consider, but I don't think there's anything wrong with the "addon package" model. That also brings up a separate point – I wrote it to demonstrate that it was possible, but I don't use named routes, and I'm not sure any of the other project collaborators do as well. While there's not much code that can break, and I sort of care as a point of pride, it might be best for this feature to be maintained by someone who actually uses it. |
@taion If you don't use named routes, what is the best way to archive paths reusage. I mean if I want to type paths in one place and use corresponding variables in places where I need it? Is https://www.npmjs.com/package/use-named-routes the best way to archive it or is there another pattern? Kind of:
|
Every single major SPA Router has this feature, the React community itself has been insisting for years now that it is an important feature, who had the idea that Named Routes is an disposable feature? At this point, the only drawback of using React is having to use React Router, a routing library that doesn't even support named routes. |
@iErik you might be disagree with this choice (I am too) however, there is a way to express opinion... also nothing force you to use this code ... also nothing force you to write a small helper fonction to generate routes. |
You can build this with something like the Route Config example from the docs: https://reacttraining.com/react-router/examples/route-config It no longer has to be baked into the core, just wrap some components with a dash of magic and voilà, you've got named routes. Something like that would make a great addon package for 4.0. I'd definitely be interested in having that live alongside react-router-dom, react-router-native, and some other stuff that's going to live here in the near future. We're looking to build a whole ecosystem of addon packages to the simple core of react-router. No more batteries included, but the batteries will be available nearby 😄 If anyone wants to champion an effort on that, please submit a PR and we can get the ball moving. |
Implementing a static route config and named routes on top of this project at this point is like a 20 line ordeal. 😂 If we baked it in we couldn't do recursive, dynamic routing. SCREW US FOR GIVING YOU A MORE POWERFUL ABSTRACTION |
There has been a lot of discussion about this already in #1514 and #1791.
We removed
<Route name>
in 1.0 for a few important reasons:getChildRoutes
) means that we may not actually be able to build real URLs for<Link>
s whose route config has not yet loaded<Link to>
doesn't take any extra time–you have to look up the route path or the name, which are usually pretty reflective of each other<Redirect>
insteadGiven the above reasons, it hardly seems worth it to bring back
<Route name>
as an API that we prescribe for everyone. However, some users believe that<Route name>
can still be useful to specify a name for a route that can be used during development.If we did bring it back, it probably work something like this:
Please, please refrain from "+1" comments here and keep the discussion to the possible benefits/drawbacks to adding this feature back in. Thanks :)
The text was updated successfully, but these errors were encountered: