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

[v6] feature suggestion: possibility to use nested routes with absolute paths #7335

Closed
smashercosmo opened this issue May 13, 2020 · 34 comments

Comments

@smashercosmo
Copy link
Contributor

I want to be able to do the following

routes.js

<Router>
  <Route path="/some-url" element={<SomeElement />}>
      <Route path="/some-url/some-other-url" element={<SomeOtherElement />}>
  </Route>
</Router>

SomeElement.js

<div>
  <h1>SomeElement</h1>
  <Outlet />
</div>

SomeOtherElement.js

<div>Some other element</div>

And if user lands on /some-url/some-other-url, he should see

<div>
  <h1>SomeElement</h1>
  <div>Some other element</div>
</div>

Motivation:
All paths in our app are defined as absolute paths like this

paths.js

export const SOME_URL = '/some-url'
export const SOME_OTHER_URL = '/some-url/some-other-url'
@mjackson
Copy link
Member

A nested route with an absolute path that does not exactly match its parent routes' paths is not valid, so any child routes that you define in this way will have to match their parent paths exactly. While we could technically enforce this, it doesn't seem like it provides much benefit.

Why not just use relative URLs?

@smashercosmo
Copy link
Contributor Author

Well, I have two reasons for not using relative URLs.

  1. First of all react-router v5 was advocating using absolute URLs, that's why they are being used across the whole app and it's quite painful to rewrite them as relative ones.
  2. Second, I personally think that absolute URLs are easier to maintain and are much clearer when reading the code. Because you can have many levels of nested routes and when you read the code of some random component and stumble upon <Link to="something" /> you can't immediately tell where this link actually leads to.

@brookback
Copy link

brookback commented Jun 5, 2020

I second everything @smashercosmo says.

Most frontend-y apps (including ours) have been built with some routes.js file with absolute paths:

/
/users
/users/:id
etc.

Rewriting all of this would be painful.

We currently would like to do something like this:

// Top level App.tsx
<Router>
  <Routes>
     <Route path="/:teamSlug/*" element={<Team />} />
  </Routes>
</Router>

// "Layout" for all things in teamSlug
const Team = () => (
  <div>
     <Sidebar />

     <Routes>
         {/* These absolute paths won't work when this is nested inside <Team /> */}
         <Route path="/:teamSlug/settings" element={<TeamSettings />} />
         <Route path="/:teamSlug/:projectSlug" element={<Project />} />
     </Routes>
  </div>
)

I'm kinda new to React Router: there might be alternate ways of achieveing this.

One of the main reasons we've got absolute routes is because of named routes with typed parameters. With Typescript, we can have a type of all routes, as well as their parameters:

// routes.ts

interface Team {
  to: 'team',
  params: { teamSlug: string },
}

interface Project {
  to: 'project',
  params: { teamSlug: string, projectSlug: string },
}

export type AppRoute = Team | Project;

// Used as: <Route path={routes.team} /> in app code
export const routes: { [name: string]: string } = {
  team: '/:teamSlug',
  project: '/:teamSlug/:projectSlug',
}
import { routes, AppRoute } from './routes';
import {
    Link as RouteLink,
    LinkProps as RouteLinkProps,
} from 'react-router-dom';

// Link.tsx

type LinkProps = AppRoute & Omit<RouteLinkProps, 'to'>;

export const Link: React.FunctionComponent<LinkProps> = ({ to, params, ...rest}) => {
   let path = routes[to]; // path=/:teamSlug

    if (params) {
        for (const [key, val] of Object.entries(params)) {
            path = path.replace(`:${key}`, (_) => String(val));
        }
    }

   return <RouteLink ref={ref} to={path} {...rest} />;
};
// app code
import { Link } from './Link';

<Link to="team" params={{ teamSlug: "foo" }}>Team</Link>
<Link to="project" params={{ teamSlug: "foo", projectSlug: "bar" }}>Project</Link>
{/* Won't compile */}
<Link to="project" params={{ teamSlug: "foo" }}>Missing param</Link>
<Link to="foo" params={{ teamSlug: "foo", projectSlug: "bar" }}>Incorrect `to`</Link>

@mjackson
Copy link
Member

mjackson commented Jun 6, 2020

I hear you about rewriting all your existing route paths. I'd like to avoid that if we can.

One case that I think will be very confusing for people is with path="/" for "index" routes. It feels quite natural in a nested route to use <Route path="/"> to indicate a nested route that matches the URL up to that point, but no more. For example:

<Route path="settings" element={SettingsPage}>
  <Route path="/" element={<SettingsIndex />} />
</Route>

In this case, it's pretty clear that path="/" means "match the /settings URL" (remembering we always ignore trailing slashes when matching paths), whereas "match the / (root) URL" wouldn't make any sense at all here.

Perhaps, since you have all absolute paths, you could dynamically generate your routes by looping over your array/object in a map?

@robertvansteen
Copy link

Perhaps this could be solved by an absolute prop on the <Route /> component? Which by default is false, resulting in the behavior as you described. But if you know what you are doing and you have a more complex routing setup the path can be treated as an absolute path like it worked in previous versions. That way you don't have to deal with certain "meanings" in the path prop like a / or a . and it's easier to see what's going on.

@smashercosmo
Copy link
Contributor Author

smashercosmo commented Jun 7, 2020

One case that I think will be very confusing for people is with path="/" for "index" routes. It feels quite natural in a nested route to use <Route path="/">

This doesn't look natural to me at all. Path, that starts with forward slash, always indicates that this is a root path. IMHO using path-less routes for index routes feels much more natural.

<Route path="settings" element={SettingsPage}>
  <Route element={<SettingsIndex />} />
</Route>

Perhaps this could be solved by an absolute prop on the component

Good idea, but I think it should be on <Routes /> or even on <...Router /> component. Something like

<Routes mode="absolute">
  <Route path="/some-url" element={<SomeElement />}>
      <Route path="/some-url/some-other-url" element={<SomeOtherElement />}>
  </Route>
</Routes>

@mjackson
Copy link
Member

What does your current route config look like in v5 @smashercosmo? Are you doing the whole thing in JSX with absolute paths?

@davnicwil
Copy link

In case this is helpful for anyone, what I'm doing is just wrapping my absolute paths in a function which converts them into the relative form expected by <Route>

const route = (absolutePath: string) => absolutePath.substr(path.lastIndexOf('/') + 1)

Keeps the readability benefit and means if you have all your absolute paths defined in one place already, you don't have to refactor that and can just use it as-is.

From @smashercosmo example, it'd look like this

  <Route path={route("/some-url")} element={<SomeElement />}>
      <Route path={route("/some-url/some-other-url")} element={<SomeOtherElement />}>
  </Route>

Or if you had paths defined in a paths constant or similar:

  <Route path={route(paths.someUrl.base)} element={<SomeElement />}>
      <Route path={route(paths.someUrl.someOtherUrl)} element={<SomeOtherElement />}>
  </Route>

Super simple and, for me, does the job. I guess it might be nice to support absolute mode as a native feature but that adds API weight and complexity, and I guess this pattern might be an acceptable alternative that already works?

@MeiKatz
Copy link
Contributor

MeiKatz commented Jun 16, 2020

I think the big problem is, that in RRv6 / is used to mark the root of some sub-route. Therefore you cannot distinguish if it points to top-level root or to the root of some sub-route. Therefore a better solution would be either the empty string or . to point to the root of a sub-route. Then RRv6 could add absolute paths without a clash of meanings.

@davnicwil
Copy link

@MeiKatz just to clarify was that in response to my post? :-)

@MeiKatz
Copy link
Contributor

MeiKatz commented Jun 16, 2020

@davnicwil Kind of. I just wanted to point out, why this change could make some confusions if done wrong.

@davnicwil
Copy link

davnicwil commented Jun 16, 2020

@MeiKatz I agree. At least how I'm thinking about it is that the trailing /, similar to *, is just a react-router specific configuration expression.

I therefore don't try to configure my 'root' components with actual paths from my paths constant, I just hard code them into the Routes like <Route path="/" ..., just as I hard code * chars like <Route path={paths.something.base + '/*'}... - I want to keep these 'special characters' out of my actual path definitions, only using them in the react-router Route definitions.

What's confusing is that, unlike *, / looks like a legit part of an actual path and appears inconsistent. I.e. why does the root path have a leading / but no other sub paths do?

I agree that a good way to solve this might be to make this 'root' special char something different, something more obviously a special char, like . as you suggest.

Weirdly though, to sidestep this completely, playing around I've noticed that in fact both leaving path out and setting path="" also seem to work for a root - see here - @mjackson I'm not sure if this is intended behaviour but if so these could also work as pretty decent alternatives?

@thedanwoods
Copy link

Are absolute paths in <Route /> likely to become a feature? It seems useful and harmless to provide some sort of absolute prop.

We have rather a lot of routes, and we define them a bit like this:

export const ROUTES = {
  HOME: '/',
  SOME_MODULE: {
    INDEX: '/some-module',
    FEATURE: {
      INDEX: '/some-module/feature',
      ITEM: '/some-module/feature/:id',
      getItemRoute: id => `/some-module/feature/${id}`,
    },
  },
};

The path and the function that builds the path are next to each other because they're two sides of the same coin: one is used for a <Route /> and the other for a <Link />. getItemRoute needs to return an absolute path, because it is meant to be used by any component, anywhere, that wants to point to /some-module/feature/:id.

It seems weird and brittle to then have to define a separate relative path for a <Route /> when the reality is you always mean an absolute path.

@thedanwoods
Copy link

@mjackson We've now spent some time trying to making this work in our app, and having to use relative paths in <Route> is not a pleasant experience.

It's very helpful to be able to say, from anywhere, <Link to={SOME_GLOBAL_CONSTANT}>, and guarantee that it points to the same place as <Route path={SOME_GLOBAL_CONSTANT}>. This pattern makes a messy real world app far easier to maintain, as we have a lot of confidence that we aren't breaking navigation.

As it is, without absolute paths as an option, <Route path="/somewhere"> and <Link to="/somewhere"> mean completely different things.

The only solutions I can think of are:

(a) define all the path constants twice, one absolute and one relative (feels brittle); or
(b) whenever we have <Routes> grab the current location.pathname and strip that off the front of the path constant (feels like a hack).

Am I missing something? Being able to say <Route absolute path={SOME_GLOBAL_CONSTANT}> would solve all this in a jiffy.

@stale
Copy link

stale bot commented Sep 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

@stale stale bot added the stale label Sep 21, 2020
@smashercosmo
Copy link
Contributor Author

@timdorr do you mind adding fresh label to the issue?

@stale stale bot removed the stale label Sep 23, 2020
@timdorr timdorr added the fresh label Sep 23, 2020
@timdorr
Copy link
Member

timdorr commented Sep 23, 2020

There you go.

@smashercosmo
Copy link
Contributor Author

With the latest TypeScript features (https://devblogs.microsoft.com/typescript/announcing-typescript-4-1-beta/#template-literal-types), using absolute urls across application becoming even more relevant. Now we can do something like this:

paths.ts

export const USER_POST_URL = '/users/:userId/posts/:postId'

UserPost.tsx

import { useParams } from 'react-router-dom'
import * as PATHS from '../paths.ts'
import type { Params } from '../types.ts'

function UserPost() {
  const { userId, postId } = useParams() as Params<PATHS.USER_POST_URL>
}

@ThomasBurleson
Copy link

ThomasBurleson commented Dec 8, 2020

I feel compelled to jump in here also. To be consistent with HTML standards and dev expectations, IMO

  • Any path starting with / should be considered as absolute (eg from the root)
  • Relative paths should NOT have a start forward slash; eg

Instead of using the current <Route path="/" element={...} />, perhaps we could do this:

  • <Route path="" element={...} /> defines an initial relative path that does not have a segment
  • <Route path=":topicId" element={<Topic />} /> defines an relative path that does have a segment match

While using absolute attribute in the Routes or Route component may work... it seems to be a hack to a conceptual problem. I go back to the idea of "What is the expected syntax for developers familiar with HTML, Angular, and other non-React frameworks?" Consistency would be the goal.

@MeiKatz
Copy link
Contributor

MeiKatz commented Dec 8, 2020

@ThomasBurleson Exactly. That's exactly the way that I would implement it. The currently use of / is just inconsistent.

@amcsi
Copy link

amcsi commented Feb 15, 2021

I would like to have support for absolute paths, because routes for me look something like this:

export const pathTo = '/path/to';
export const route1 = `${pathTo}/my/route1`;
export const route2 = `${pathTo}/my/route2`;

With the above, I could e.g. use route1 for both <Route path={route1} />, and <Link to={route1} />, and in my IDE, both will point to the same source.

If I were forced to use relative paths for route configuration, then I would have to do something like this to try to be as DRY as possible:

// root path
export const pathTo = '/path/to';

// subpath definitions
export const route1SubPath = `/my/route1`;
export const route2SubPath = `/my/route2`;

// full path definitions
export const route1FullPath = `${pathTo}${route1SubPath}`;
export const route2FullPath = `${pathTo}${route2SubPath}`;

And using the previous example with Route and Link, I would need to do <Route path={route1SubPath} /> for routes (because they are relative), but <Link to={route1FullPath} /> when using links. Now the two constants do not point to the same thing, and I won't be able to find usages of both routes and links with the help of a single constant.

(Though it would make things better if react-router provided a tool to take a nested route configuration and spit back an amended configuration that would contain both the absolute and relative path for each route, because then in my IDE I could just find usages of each route object that would cover both the relative and absolute paths.)

@sebastian-nowak
Copy link

Any updates on this? Beta version is already out and it's a very important feature that's still missing.

I'm implementing i18n, my URLs are translated and I have to work with absolute addresses, because the translation library doesn't have access to react router context. Adding workarounds for this makes everything super messy.

@renatobenks
Copy link

renatobenks commented Jul 28, 2021

agreed, relative paths are making navigation-routes definition too messy, because the path definition need to be composed by parent routes path to expose a "navigately" route 😕

@andykant
Copy link

It would be great to support absolute routes based on @ThomasBurleson's suggestion.

I recently upgraded a large codebase to v6 beta (partially because we needed the history improvements), and this change made it mandatory to refactor/reintegrate how we used routes throughout the app from v5. I like the new pattern, but it's more opinionated and enforces that your codebase must be organized for relative paths.

Here are a couple of our use cases where having absolute routes is useful, or at least enables versatility:

  • Displaying conditional route-specific content in a reusable/shared component.
  • Applying additional route variants to a more generic parent route.

@mjackson
Copy link
Member

Thanks everyone for chiming in here. I appreciate all the conversation.

I definitely hear what you're saying re: following existing URL conventions @ThomasBurleson. i.e. starting with a / indicates an absolute URL and not starting with a / indicates a relative one. That's exactly what we do with <Link>. But <Route path> is a little different.

My main hangup with including support for absolute URLs in nested <Route path>s is that it makes it possible to introduce errors, since every nested route must start with the entire path of all its parents. A nested route with a different absolute URL prefix than its parent routes would be unreachable since its parent route would never match.

That's why we built this feature the way we did, because it saves you from having to type out all your route paths from the root every time.

For example, assuming we did support absolute paths in nested routes:

<Routes>
  <Route path="users" element={...}>

    {/* THIS IS AN ERROR, since the parent route path will never match
        a URL that begins with /customers */}
    <Route path="/customers/:id" element={...} />

    {/* Since the above route MUST start with /users or be
        an error, it seems easier to just leave off the URL prefix */}
    <Route path=":id" element={...} />

    {/* Or if you really want it you could use an absolute path.
        Just be sure it starts with all of its parent route paths! */}
    <Route path="/users/:id" element={...} />

  </Route>
</Routes>

I get that most people who want this feature are probably using constants for their route paths instead of writing them out by hand, but the point still stands. A nested route with an absolute path must duplicate all path segments of its parent route or it will be unreachable.

However, in the current architecture where all routes are relative to their parents it is not possible to create a nested route that is not reachable.

I'm not saying that we absolutely cannot support this feature, but I am wondering what is the actual use case here? Because if we're going to introduce a feature that opens up the potential for errors, we better have a good reason.

@andykant Would you be willing to expand on your use cases a little? I'm not quite sure what either of those mean. Some example code would be great.

Thanks, everyone for your patience!

@MeiKatz
Copy link
Contributor

MeiKatz commented Aug 30, 2021

@mjackson I think we have two problems in here:

  1. make the definiton of path in <Route /> consistent with to in <Link />.
  2. support absolute paths.

The 1st point should be easy and should be done right now.

The 2nd point could be implemented for those who use the feature unless there is a better solution that enable the use of absolute paths constants.

@amcsi
Copy link

amcsi commented Aug 30, 2021

@mjackson You are right about it being logical for Routes to ideally be relative paths. But my biggest issue is the lack of possibility to use some sort of shared reference between Routes and Links so that in your IDE you can easily find which Links point to which Routes and vice versa (since in React Router 6 you'd be forced to use separate string literals/constants between Links and Routes). In React Router 5, although not logical for the reasons you said, I was at least able to share absolute path constants between Routes and Links; now not anymore with RR 6.

It would be great to have some tool like the following either built into or endorsed by React Router 6:
https://github.com/fenok/react-router-typesafe-routes
https://typehero.org/type-route/docs/introduction/simple-react-example

That way the problem regarding shared references would be solved.

@mjackson
Copy link
Member

mjackson commented Aug 31, 2021

make the definiton of path in <Route /> consistent with to in <Link />

@MeiKatz <Route path> and <Link to> are two different things:

  • <Route path> is a pattern to match against a portion of the URL pathname
  • <Link to> is a URL

The 1st point should be easy and should be done right now.

With respect, this is naive. This change would most likely break ALL apps using React Router v6 since any nested <Route path="/"> would now be broken. In other words, adding support for using absolute <Route path> values in nested routes also means we implicitly remove support for a nested /something-relative, so anyone who relied on that behavior up to this point would now have a broken app.

So although this change might be easy to make in our code, it would be a difficult one for the community. The only reason I think it might still be possible to make is because we are still in beta on v6. But I know for a fact that many people are already using v6 in production, despite it not being ready yet.

my biggest issue is the lack of possibility to use some sort of shared reference between Routes and Links so that in your IDE you can easily find which Links point to which Routes and vice versa

I'm guessing this is how you would do this in v6 if we supported nested absolute paths, @amcsi:

let USERS_PATH = '/users';
let USER_PATH = '/users/:id';
let USER_CREATE_PATH = '/users/new';

// routes
<Routes>
  <Route path={USERS_PATH} element={<Users />}>
    <Route path={USER_PATH} element={<UserProfile />} />
    <Route path={USER_CREATE_PATH} element={<UserCreate />} />
  </Route>
</Routes>

// links
<Link to={generatePath(USER_PATH, { id: 123 })}>User 123</Link>

Is that close?

@MeiKatz
Copy link
Contributor

MeiKatz commented Aug 31, 2021

You said it: RRv6 is in still in beta and the reason why some people use it in production is, that the development of this next major version stopped for a long time. People migrating from v5 to v6 have to change there code anyway.

What I meant with consistent behaviour: relative paths in <Link /> should be relative to the most inner route they appear, absolute paths should point the root of the router. Therefore relative paths in <Route /> are relative to the their closest predecessor, absolute paths absolute to the root of the router. This would make re-using components a lot easier.

@amcsi
Copy link

amcsi commented Aug 31, 2021

@mjackson yes that is pretty much how I would do it if you supported nested absolute paths. With that I can find usages of the USER_PATH constant in my IDE and find the route and all the.

I may in addition assemble USER_PATH from USERS_PATH to be DRY and lessen the chances of writing invalid nested absolute routes:

const USER_PATH = `${USERS_PATH}/:id`;

@thedanwoods
Copy link

thedanwoods commented Aug 31, 2021

@mjackson Right now, because we define all path strings in a single file, we're having to do stuff like this:

let USERS_PATH = '/users';
let USER_PATH = '/users/:id';
let generateUserPath = id => ({ id }) => `/users/${id}`;
let RELATIVE_USER_PATH = ':id';
let USER_CREATE_PATH = '/users/new';
let RELATIVE_USER_CREATE_PATH = 'new';

// routes
<Routes>
  <Route path={USERS_PATH} element={<Users />}>
    <Route path={RELATIVE_USER_PATH} element={<UserProfile />} />
    <Route path={RELATIVE_USER_CREATE_PATH} element={<UserCreate />} />
  </Route>
</Routes>

// links
<Link to={USER_CREATE_PATH}>Create a user</Link>
<Link to={generateUserPath({ id: 123 })}>User 123</Link>

This is not very nice, and perhaps isn't the most elegant way to solve the problem. But as we want to define all our paths in the same place, while also having an app-level <Routes> component that renders module-level <ModuleRoutes> components and so on, it's problem we need to solve somehow.

The awkward RELATIVE_ stuff is only needed because the <Route path={}> cares about where in the tree it's rendered.

If you're defining all your path constants in the same place - which I think is a fairly common practice - having to do fiddly workarounds actually increases the risk of unreachable routes. It would be simpler and more robust if <Route> provided some way to accept absolute path strings.

@mjackson
Copy link
Member

mjackson commented Sep 1, 2021

Thanks everyone for the discussion here, and for helping us to better understand your use cases. We've decided to go ahead and add support for nested absolute paths in v6. Hopefully this makes it easier for you to define your route paths in a separate file and track where they are being used in your app.

Please be aware that if you are currently using the v6 beta any absolute paths in nested routes will throw if they do not begin with the combined path of all their parent routes.

Will follow up in #7992.

chaance pushed a commit that referenced this issue Sep 2, 2021
- Nested routes with absolute paths that do not match their parent
  path are now an error
- Also adds <Route index> prop for index routes

Fixes #7335
chaance pushed a commit that referenced this issue Sep 2, 2021
- Nested routes with absolute paths that do not match their parent
  path are now an error
- Also adds <Route index> prop for index routes

Fixes #7335
@andykant
Copy link

andykant commented Sep 8, 2021

@mjackson

Thanks everyone for the discussion here, and for helping us to better understand your use cases. We've decided to go ahead and add support for nested absolute paths in v6. Hopefully this makes it easier for you to define your route paths in a separate file and track where they are being used in your app.

Awesome, thank you!

@andykant Would you be willing to expand on your use cases a little? I'm not quite sure what either of those mean. Some example code would be great.

Sure! To be clear, the main point that I was raising is that these are patterns that were available in v5 and require non-trivial refactors/testing in order to upgrade to v6. These can still be done in v6, but the v6 version has pros (more explicit) and cons (more boilerplate).

Displaying conditional route-specific content in a reusable/shared component.
I wouldn't describe this as common, but it's nice to have the ability available. A couple examples:

  • Displaying privileged information when the URL is prefixed with /admin, that should otherwise be hidden in case the page is being screen-shared on a video call.
  • A generic breadcrumb component that is route-aware but needs to be mounted inside a deep child component.

Applying additional route variants to a more generic parent route.
Our specific use case here is that we have a id-like path, but the id could point to a user-configured view or a reserved name for a pre-configured view (that provides a similar user experience but integrates different components).

v5 pseudocode:

root.js
  <Route path="/dashboard/:id/:name?/:options?" component={DashboardsWrapper} />

Dashboards.js
  ...
  <Switch>
    <Route path="/dashboard/summary" component={SummaryDashboard} />
    <Route path="/dashboard/:id/:name?/:options?" component={CustomDashboard} />
  </Switch>
  ...

@mjackson mjackson removed the fresh label Sep 9, 2021
mjackson added a commit that referenced this issue Sep 10, 2021
Also:

- Add (optional) `<Route index>` prop for index routes
- Return original route objects from `matchRoutes`
- Remove `PartialRouteObject` interface, use `RouteObject` instead
- Remove `createRoutesFromArray`

Fixes #7335
mjackson added a commit that referenced this issue Sep 10, 2021
Also:

- Add (optional) `<Route index>` prop for index routes
- Return original route objects from `matchRoutes`
- Remove `PartialRouteObject` interface, use `RouteObject` instead
- Remove `createRoutesFromArray`

Fixes #7335
@mjackson
Copy link
Member

This was fixed in #7992. It will be released in v6 beta 4 later today.

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

No branches or pull requests