-
-
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
Should we keep JSX configuration? #865
Comments
[changed] Route names are nested This commit formalizes and enhances two of the core primitives in the router: Route and Match. We get a few benefits from this: 1. Routes may now be created programmatically, as well as via JSX. This is useful in situations where it is desirable to assemble the route configuration using separate modules, instead of all at once. For example, in ApplicationRoute.js you could have: module.exports = Router.createRoute(); and in UserProfileRoute.js: var ApplicationRoute = require('./ApplicationRoute'); module.exports = Router.createRoute({ parentRoute: ApplicationRoute, path: 'users/:id' }); 2. <Link to> may reference a Route object directly. <Link to={UserProfileRoute}> 3. Route names may be re-used at different levels of the hierarchy. For example, you could have two different routes named "new" but nested inside different parent routes. <Route name="users" handler={Users}> <DefaultRoute handler={ShowAllUsers}/> <Route name="new" handler={NewUser}/> </Route> <Route name="posts" handler={Posts}> <DefaultRoute handler={ShowAllPosts}/> <Route name="new" handler={NewPost}/> </Route> Using this route configuration, you could <Link to="users.new"> or <Link to="posts.new"> depending on which one you wanted. A side effect of this is that names of nested routes are no longer "global", so e.g. <Link to="new"> won't work because it is ambiguous, but <Link to="posts"> will still work.
I really like having the ability to define all my routes in a single place with jsx. Both apps I have in production have around 40 routes, and I haven't felt any pain in maintaining a |
@rickharrison what if you just linked to the path? I find my route names end up just being truncated versions of the path, could conventionally say "foo/:id/bar/:name" becomes |
Perhaps I'm missing something, but why drop |
One benefit of getting rid of <Link to="same/:path/as/:route/config" params={params}/>
<Link to={RouteObject} params={params}/> Then we wouldn't need context at all to generate links. Any chance to not rely on context I'd love to take. I also like how simple it is to |
@taurose Great observation. Yes, that's correct. I preserved @ryanflorence I'd rather not introduce yet another notation for paths. We already have URL patterns that have special notation. Another special notation is confusing, IMO. |
@ryanflorence Yes, that's a great point too and IMO the strongest argument in favor of not supporting string names. But I also think that @taurose has a great point: if we get rid of |
@ryanflorence I don't like linking full paths all throughout the code base, because there have been instances where I change the URL for a particular resource. Then I just have to change it in one place and add a simple For me, I just wouldn't want to add 40 more files to my project all for things which are just one line of configuration. |
@rickharrison I hear you. That's exactly how I felt when I went to refactor all of our example apps to not use |
Yeah, that was my original intent on |
Just keep name as it works today, they're all just global references, don't concat with parents or anything. |
whah what the heck, there are two issues here? all of these comments were for #865 |
wait, what is going on? why the topic change? keeping "name" and ditching "jsx" are different things. |
@ryanflorence See @taurose' comment above #865 (comment) |
I'm opening this issue to discuss whether or not we want to keep
<Route name>
. In c5a24a5 I introduced theRoute
class which lets you use aRoute
object directly in<Link to={MyRoute}>
. This is nice when you have many, many routes that you can't keep all in one file.Technically, this eliminates the need for
<Route name>
because you can just use object references. Indeed, I started the work on c5a24a5 excluding names altogether. However, when I went back to make the examples work with the new object reference style, it didn't feel nearly as convenient as just using a string name. I don't want people just getting started with the router to have to go through the ceremony of creating a bunch of object references, when a fairly high % of our users (arguably most) will get by just fine using only route names.I can also understand people being bothered by the fact that there's more than one way to link to stuff. But it doesn't bother me enough to remove the feature entirely.
/cc @ryanflorence @taurose @josephsavona
The text was updated successfully, but these errors were encountered: