remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
52.76k stars 10.21k forks source link

RFC: Relative Links and Routes #5127

Closed ryanflorence closed 4 years ago

ryanflorence commented 7 years ago

We intend to ship Relative Routes and Relative Links soon. I'd like to hear people's comments on something I'm not totally sure about yet.

What are Relative Links and Routes?

Instead of <Route path={match.path + '/more/stuff'}/> you could simply do <Route path="more/stuff"/> and Router will use the lack of / at the start of your path as a hint that you want to just use whatever path is above you. Relative!

Same with links. Instead of <Link to={match.url + '/more/stuff'}/> you can just do <Link to="more/stuff"/>.

The Problem

Going "down" as shown in the quick example up there is easy. But we're not sure what ".." should mean. There are two options for what it means: go up a URL segment, or go up a matched route.

Go up a URL segment

With plain anchors tags, .. means "up a URL segment". Given the following URL, an anchor tag with the following HREFs results in the following URLs:

/clients/invoice/123
href url
payments /clients/invoice/123/payments
/clients/invoice/123
. /clients/invoice/123
.. /clients/invoice
../ /clients/invoice
../.. /clients
../../ /clients

(As far as I can tell, feel free to double check!)

So I think everybody's intuition around this is to just do exactly that and be done with it. Ship it.

However, I have this gut instinct that our intuition right now might be keeping us from a better meaning of ".." in a React Router app?

Go up a matched Route

Maybe your app is something like this:

  <Switch>
    <Route exact path="/" component={Home}/>
    <Route path="/clients" render={() => (
      <Clients>
        <Switch>
          <Route exact path="." component={ClientList}/>
          <Route path="invoice/:invoiceId" component={Invoice}/>
        </Switch>
      </Clients>
    )}/>
  </Switch>

Maybe we want ".." to mean "go up a matched route" rather than "go up some url segments".

So now the "relative" portion of the Link's to prop is the parent Route's path, not the current location.

to prop link's url
.. /clients
../.. /

Notice .. doesn't go to "invoice" because that's not a route that will ever match.

Pros

Cons

What do you think?

davidascher commented 7 years ago

I think the expected behavior is better because expected with that syntax. The route walking behavior could be useful but I'd suggest looking for another syntax.

ryanflorence commented 7 years ago

Ah so maybe <Link to="../projects"/> works on URL segments and <Link to="^^/projects"/> could work on the parent route's path. Interesting.

Not excited about inventing syntax though.

alanhussey commented 7 years ago

What about a separate prop that is mutually exclusive with to? Something like <Link upOneRoute>? Then you can avoid overloading the API. (another option: <Link up={count}>, to traverse more than one level)

1000hz commented 7 years ago

I think going up a matched route would be a bad idea. 2 reasons I can think of off the top of my head:

Notice .. doesn't go to "invoice" because that's not a route that will ever match.

  1. I don't think you can make this claim when routes in v4 can be anywhere in the component tree.
  1. What happens when you decide to add a <Route path="invoice" /> a few weeks later? Now you've inadvertently borked all your relative links.
BTMPL commented 7 years ago

I think your assumption in first post is wrong, @ryanflorence

Given URL like /clients/invoice/123 the behavior is:

href url
payments /clients/invoice/payments
` (emptyhref` attribute) /clients/invoice/123
. /clients/invoice
.. /clients
../.. /
pshrmn commented 7 years ago

@BTMPL Resolving here is going to be slightly different than how the browser treats hrefs. Before stripping the last segment, we ensure that the pathname ends in a trailing slash. That way only an empty string is stripped and we can treat /clients/invoice/123 the same as /clients/invoice/123/ (we get the segments ['clients', 'invoice', '123']).

The only tricky thing with that is that we have to keep track of if the original path had a trailing slash. I dealt with that in #4459 with this code and these tests (which I think are correct, but I haven't reviewed them in a couple months).

BTMPL commented 7 years ago

@pshrmn I understand that this is a decision aimed at new developers that might be unfamiliar with the URI syntax, but that's a step away from the specs.

Also, Ryan's example was based on anchor tags, so I assumed spec compliant, not react-router variant and should follow the RFS - https://tools.ietf.org/html/rfc1808#section-5

Is there a real need for relative links other than not having to access props.match.url ?

mcrawshaw commented 7 years ago

The composability of React and RR4 are great. I think when @ryanflorence mentions the second option he is targeting 1st class support for this concept. Allowing a component to ignore implementation details of its owner/parent, etc.

Conceptually, I think it's useful to classify path segments in 2 seperate ways. Navigation and parameter. An example /list/page2/item1/tab2. For the sake of discussion, list and item1 are probably navigation segments (resulting in route matches and a rendered component), and page2 and tab2 are parameter segments (passed to the matching component). I argue that navigation segments are often what we want to work with when discussion relative paths. Parameter segments are often optional and can cause headaches.

I'm not sure what the exact solution is, but my most common needs are as follows:

I believe this description keeps cohesion within the component.

The unfortunate thing about this discussion is that you might have a completely different view if you use RR4 differently within your app. But hoping this makes sense to as most people as possible.

thomasvm commented 7 years ago

In my head <Link to="" /> directly corresponds to <a href="" />, so I would the expect the to prop to behave in the same way as the a's href. If a relative link behaves differently in react-router than the html counterpart, then that is something and you'll probably will be explaining over and over and over again.

Could the alternative behaviour be implemented in a dedicated component?

mcrawshaw commented 7 years ago

I agree @thomasvm, except for needing a separate component.

Especially with .. as it has such an intuitive use (remove a single segment from the current url). Changing it would require massive userland instruction.

I don't believe . has the same familiarity.

I think @davidascher suggestion is worth considering.

pshrmn commented 7 years ago

Relative links would be completely for convenience (i.e. not having to use match.url and string concatenation).

I think that as long as the resolving behavior is well documented, I'm okay with being slightly different from how anchors resolve. The current URI spec seems mostly inspired by static file servers, where trailing slashes are normal (e.g. /somewhere/ is really /somewhere/index.html). When they aren't, linking to child pages is annoying.

<!--
for example, with window.location.pathname = '/user/123',
you have to include the user id in a relative link
-->
<a href='123/edit'>Edit</a>

If we automatically insert a trailing slash, then we get the behavior that I feel like the resolving was originally built around. Maybe I'm just assuming that intention because it fits my argument, but RFC 1808 was written over 20 years ago.

Also, while this behavior is different from how anchors resolve, it is consistent with how the terminal apears to resolve with cd (they may be appending the trailing slash, but visually the terminal does not show it).

~/code $ cd forks
~/code/forks $ ls
react-router history
~/code/forks $ cd react-router
~/code/forks/react-router $ cd ../history
~/code/forks/history $

Anyways, I'm more of a fan of the segment-wise double dot resolving because it is more predictable. I can certainly see the appeal to match based resolving, but I feel like that might lead to weird edge cases.

BTMPL commented 7 years ago

I would be a fan of this solution if react-router would enforce traling slashes but I don't think enforcing anything like that is what the devs want. Maybe adding a separate link component or a prop on the link component to change it to relative mode is the solution?

Maybe I'm just assuming that intention because it fits my argument, but RFC 1808 was written over 20 years ago.

Sure, but it's still in use today, and a lot of webdevs are used to how it works ;)

lewisdiamond commented 7 years ago

@ryanflorence Given /clients/invoice/123, a link to payments should definitely go to /clients/invoice/payments, not what is suggested in the first post. @BTMPL Given /clients/invoice/123, . should not go to /clients/invoice but rather /clients/invoice/ (trailing slash, same goes for your .. example)

Why not follow standards?

binki commented 7 years ago

I came to this bug researching how to write relative <Route/> with react-router because I don’t get how nesting would make sense without support for relative routes. I was surprised that this is not a feature yet.

Maybe links could just not support ... If you’re trying to navigate to a different page in the app which requires going up, that means your <Link/> implies it knows the structure of its parent components. Requiring components to know where they will be called from seems to work against the ideal react components data flow.

By disallowing .., you could encourage in the docs for authors to pass down absolute link generators which may themselves be built from relative contexts. For example, a demonstration of a component knowing only about <Routes/> it manages and passing the necessary information to other components which might want to link to routes it manages:

<Route path='a/:aParam1/:aParam2' render={routeProps => <SubComponentA aParam1={routeProps.match.params.aParam1} aParam2={routeProps.match.params.aParam2}/>}/>
<Route path='b' render={routeProps => <SubComponentB buildALink={(aParam1, aParam2) => routeProps.match.url + '/' + ['a', aParam1, aParam2].map(encodeURIComponent).join('/')}/>}>

I think this sort of pattern enables you to write components that can use <Route/> without knowing about the structure of any components containing them. If you change any <Route/> in this component, you can immediately see all dependencies on that <Route/> and fix your link builders from one spot. This pattern is not enforced if the concept of .. is supported by relative <Link/>s because developers will take that shortcut.

Sorry if this isn’t coherent or isn’t quite on topic. My demo also probably uses react-router incorrectly.

P.S., I also find it confusing that baseUrl=/clients/invoice/123 with url=payments resolves to /clients/invoice/123/payments. Everyone expects browser/standard URI behavior such as is supported by all the URI libraries out there:

> require('url').resolve('/clients/invoice/123', 'payments')
'/clients/invoice/payments'
> require('url').resolve('/clients/invoice/123/', 'payments')
'/clients/invoice/123/payments'

However, route-relative URI support for <Link/> needs to be supported somehow for the feature to be useful. I’d like if a relative <Link/> were to always be relative to the full matched route plus a /. As long as the documentation for relative <Link/> clearly states that it is relative to matched route plus /, I think that it won’t be too confusing. That way it would match the behavior of relative <Route/>, making it easy to build links to relative <Route/>. Just don’t tell people that relative <Link/> it is relative to the URI they see in their browser, say there’s an implicit / and ensure it’s easy to get the version of the URI with that implicit / from the match object that, e.g., includes optional parameters (by just placing extra consecutive /) if there isn’t already an API for that (match.url does not do this, so it is unusable for building relative links).

binki commented 7 years ago

I realized I made a mistake in my hypothetical relative <Route/>s demo, sorry for spam. In rendering SubComponentB I need to refer to this.props.match when generating the URI builder, not routeProps.match. This way, the URI is relative to the example component rather than to my b route. Here’s the corrected version:

<Route path='a/:aParam1/:aParam2' render={routeProps => <SubComponentA aParam1={routeProps.match.params.aParam1} aParam2={routeProps.match.params.aParam2}/>}/>
<Route path='b' render={routeProps => <SubComponentB buildALink={(aParam1, aParam2) => this.props.match.url + '/' + ['a', aParam1, aParam2].map(encodeURIComponent).join('/')}/>}>
mcrawshaw commented 7 years ago

@binki I struggled a little to understand your example, but I agree with your point that we don't want sub-components making assumptions about their parent. As for removing a relative '..' as an option, that would be a little draconian for me.

@ryanflorence Have you had a chance to read the above comments? Has your thinking progressed since opening this issue?

pshrmn commented 7 years ago

The fact that history already supports relative pathnames (I guess I knew that it does, but I never see it used, so I wasn't thinkingn about it before) means that there could be inconsistent behavior for relative links. Of course the point of React Router adding relative links is that we produce an absolute URI, but there isn't a great way to infer the difference strictly looking at the pathname string.

/* current */
// location = { pathname: '/user/12345', ... }
// match = { path: '/user/:id', url: '/user/12345', params: { id: '12345' }, ... }
<Link to={`${match.params.id}/details`}>User Details</Link>
// <a href='/users/12345/details'>User Details</a>

demo

With that approach, you still have to use the match properties to generate the relative link. For me, that defeats the point of implementing relative links.

The two solutions that come to mind are to make a separate <RelativeLink> component or to add a prop to <Link> to differentiate between spec relative routing and segment relative routing. If we chose the former we would also need to write a <RelativeNavLink> which is not ideal.

Instead I think that we could add a spec prop for people who want relative paths to be resolve according to spec. You could also have a relative prop that does segment relative routing when true. Both approaches have their benefits, and maybe I'm just being stubborn, but I would prefer to have segment relative routing be the default. I think that overall it looks cleaner.

// location = { pathname: '/user/12345', ... }
// match = { path: '/user/:id', url: '/user/12345', params: { id: '12345' }, ... }

// when adding additional segments, it is simpler to just write
// the desired segment to add
<Link spec to={`${match.params.id}/details`}>User Details</Link>
<Link to='details'>User Details</Link>
// both produce <a href='/users/12345/details'>User Details</a>

// when going to different segment on the same level, it is slightly simpler to
// just write the segment value
<Link spec to='67890'>Other User</Link>
<Link to='../67890'>Other User</Link>
// both produce <a href='/users/67890'>Other User</a>
mjackson commented 7 years ago

AFAIC we've already got a great story around relative <Link> and <Route> just using the match object.

// Relative links build their to value using match.url
<Link to={match.url + '/sub/path'}>relative link</Link>

// Relative routes build their path using match.path (or match.url if they don't care about parent route's params)
<Route path={match.path + '/sub/path'} .../>

I agree these aren't perfect yet because the string concat can get messy when there's a trailing / in the URL, but it's very explicit which I like.

sohrabsaran commented 7 years ago

Avoiding match.url in case of <Route path={match.url+'/sub/path'} will be great.

Instead of the above, if we were able to just say:

<Route path='sub/path' matchEnd

...it will be easier (where 'matchEnd' is a keyword that the 'Route' component can read and process).

I ran into this 'path does not match unless prefixed with match.url' issue and resolved it through a careful look at https://reacttraining.com/react-router/web/example/basic

I then went about removing all explicit pass-downs of props via:

render()
{
    return <outerComponentTag {...this.props}  >  ...   </outerComponentTag> 

(Before going through the example in detail, I had earlier tried to do prop pass-downs to check whether they somehow resolved the issue of the Route component not working).

....but now after the cleanup of these explicit/brute-force prop pass-downs (they were generating errors in the browser console about unwanted props being passed to various components), the component where I render my Route, now complains that it cannot find 'match'!!!

svicalifornia commented 7 years ago

Back in May (sorry, I'm late to this party), @lewisdiamond suggested that . should go up a single URL segment but leave a trailing slash:

Given /clients/invoice/123, . should not go to /clients/invoice but rather /clients/invoice/

I think he's right. This would mirror how regular web links work. As he also suggested, .. would go up yet another level (e.g. /clients/), still leaving a trailing slash.

I also see and desire the benefit of going up a whole matched route, as @ryanflorence proposed. But rather than using .. for that (which would break people's expectations of how relative paths work), I suggest that we use ... to traverse up a whole matched route.

In summary, my proposal would be:

If we leave trailing slashes for . and .. as proposed above, then routes should also have an option (perhaps true by default?) to allow exact to match with or without a trailing slash.

sohrabsaran commented 7 years ago

@svicalifornia not too sure whether '.' being used to remove part of the url matched so far, would really be an intuitive syntax in general... Regarding the '...' syntax suggestion, do you mean to say that in place of:

<Route path={match.url+'/sub/path'}

...we should be able to say:

<Route path={'.../sub/path'}

...? If that's the case, then I like the suggestion; it looks nicer than the 'matchEnd' keyword suggestion I mentioned previously.

svicalifornia commented 7 years ago

@sohrabsaran Not quite. As @ryanflorence proposed at the top of this thread:

Instead of <Route path={match.path + '/more/stuff'}/> you could simply do <Route path="more/stuff"/>

So you wouldn't need any prefix for that. Just simply use "relative/path".

My suggestion of ... was in response to his request for comments:

Going "down" as shown in the quick example up there is easy. But we're not sure what ".." should mean. There are two options for what it means: go up a URL segment, or go up a matched route.

(See his post at the top of this thread for more context.)

My proposal (echoing @lewisdiamond) is that . and .. should behave exactly as web links have worked for the last ~25 years:

And to support the use case offered by @ryanflorence to go up a whole matched path, I propose that we use ...:

For example, imaging we have a route /users with a component of Users. We would write that as:

<Route path="/users" component={Users} />

Now imagine that Users contains a long Switch of many user routes, including /users/:id/profile/relationships. Since we have already matched /users in the parent route, we could write a relative route like this:

<Route path=":id/profile/relationships" component={UserRelationships} />

Then inside the render function of UserRelationships, we might want to link back to Users. So we could use ... to remove the last matched route (:id/profile/relationships), returning us to /users:

<Link to="...">Users</Link>

Or we could link to a different user's profile:

<Link to={`.../${familyMember.id}/profile`}>{familyMember.name}</Link>

This is a somewhat contrived example, and you might structure the routes differently, but hopefully this shows how ... would work and the benefit of having it in addition to . and ...

svicalifornia commented 7 years ago

@sohrabsaran If you still doubt that . should behave as @lewisdiamond and I propose, then try this in any web page:

<a href=".">Click Here</a>

Open that file locally in Chrome and click the link, and you'll see that it removes the filename from the URL and takes you up to the directory path. It works the same way on a web server, and the web has worked that way since the 1990s.

Following the existing conventions for relative paths in URLs will minimize developer confusion and increase adoption of this feature (as well as React and React Router). It's the right way to go.

And with ..., we can still traverse up matched routes and have the best of all worlds!

sohrabsaran commented 7 years ago

@svicalifornia thanks for summarising the context of the thread. I came here while looking for a way to avoid having to mention match.url in '<Route path='. I saw the last answer from @mjackson, that may have confused me about the exact scope of the opening post. I'll be more careful next time. So far I've avoided the need for '<Link to=' by using a custom component that calls history.push().

Agreed that 'no prefix' quite logically and concisely means that the given path is relative to match.url (the part of the url matched so far).

Further, as suggested in the opening post, a Link's 'to' attribute when a relative path, is relative in the exact same way as a Route's 'path' attribute when a relative path. They are both considered as prefixed with match.url.

So, I feel that it is logical and intuitive to consider a starting '..' as removing the end from match.url. I can't seem to visualise any alternate interpretation! In summary, per my intuition:

<Link to="../sub/path"

...resolves to:

<Link to="/match/dot/url/../sub/path"

...that in turn resolves to:

<Link to="/match/dot/sub/path"

This is like the expected behavior of the 'cd' command on linux; given that the current directory contains two directories 'a' and 'b', if we say

cd a/../b

...we see that the current directory changes to 'b'. This may be tried out at a linux command prompt.

So @svicalifornia, can the following concern in the original post please be elaborated with an example (I take it that unlike me, you've understood the concern):

Cons

  • *Since any ol' route can match a location w/o a relationship with other Route's that match, a Link in one place can potentially have a completely different href than a link somewhere else--even though they have the same to prop :grimacing:. It all depends on the Route's path the Link is rendered within.
  • It's against most people's intuition

...?

Do note that in the examples you've given, examples were given for '...' but not for '..'. Sorry for the bother, just trying to help to close out this one...

svicalifornia commented 7 years ago

@sohrabsaran In traditional web links, the behavior of . and .. at the start of a link href depends on the current page URL, specifically whether the current page URL ends in a slash:

If the current page URL is then . goes to and .. goes to
/some/directory/page.html
(file path; no trailing slash)
/some/directory/ /some/
/some/directory/
(directory path; trailing slash)
/some/directory/ /some/
/some/directory
(directory path; no trailing slash)
/some/ /

When the current URL path ends in slash, . goes to the same path, and .. removes the last part of the path, as you said. However, when the current URL path does not end in slash, then . removes the last part of the path, and .. removes the last two parts of the path.

This behavior of relative links is defined in Internet standard RFC 1808, published in 1995. Netscape Navigator and Internet Explorer implemented relative web links this way back in the '90s, and web browsers have handled relative links exactly the same way since then.

For traditional web sites, HTTP web servers usually take care standardizing directory URLs by redirecting any request to a directory URL without the trailing slash to the same URL but with the trailing slash added. This has the effect of adding a trailing slash to the web browser's address bar, so that all the relative links on the page will behave according to the URL ending in a trailing slash. This makes it easier for web developers to know how their relative links will work, because directory URLs will always end in a slash.

It would be great if React Router could do something similar. While our routes don't correspond to directories on disk, they often do correspond to collections of items in a similar semantic sense. For example, we would expect a route with path /products to render a component listing a collection/directory of products. Semantically, this is very similar to a directory in the traditional web. Since we are using URL routing (with the same URL syntax defined by the traditional web), then we should probably use trailing slashes in our collection/directory routes, as in /products/.

React Router could standardize our collection/directory URLs for us in much the same way that HTTP servers do, via a new Route property called collection or directory. This property would allow collection routes to match with or without the trailing slash and automatically add the trailing slash to the match URL (and perhaps to the browser's address bar, using history.replace).

For relative links to be successfully adopted in React Router, they should behave exactly as relative links do on the traditional web, as defined in RFC 1808. To simplify this and avoid having to think about whether match URLs have trailing slashes, we should add a new collection / directory property on Route so that it can add the trailing slashes for us on the appropriate routes.

Following these conventions will ensure that we are implementing relative links the same way that they have worked for over 20 years. Changing the expected behavior, even slightly, will introduce confusion and stumbling blocks for current and new developers alike, and I see no reason to do things differently when the existing conventions are known by millions of people and fairly easy to implement in React Router.

The use case offered by @ryanflorence to go up a whole matched path (which may be one or more URL parts, absolute or relative to its own parent route) has no analogue in the traditional web. Therefore I think React Router should introduce a new path prefix ... for this particular behavior. Many developers will never use this, since they are already familiar and happy with .. and how it works on the web. But I can also see myself wanting to traverse up to the route of the parent component, without having to worry about how many URL parts were matched from there to the current component, and ... would be a great addition to allow us to make that traversal with confidence.

adamdonahue commented 7 years ago

Keep existing URL semantics is my vote.

sohrabsaran commented 7 years ago
timdorr commented 7 years ago

Can you guys take this conversation elsewhere? This isn't doing much to help move the conversation along about relative routes and links. I'm going to hide your comments from the thread so they don't cloud up the discussion with a Wall of Text™.

timdorr commented 7 years ago

As to the discussion at hand. Whether the convention is ^ or ..., we're still introducing new syntax. That might be more acceptable in the case of a named-route-based approach, like we used to have back in the day. But given that we've switched to a more URL-based approach, I think we should stick to plain RFC 1808 semantics and operate on the URL level.

At some point later, we can add on a new syntax to navigate routes, instead of path segments. And in the meantime, this should be possible through to-as-a-function via #5368. I'd rather figure out the the details through a more flexible approach before locking something in that would be painful to change later on.

So, relative URLs for now; relative routes for later.

timdorr commented 7 years ago

Also, because this will make it drastically easier, @mjackson already has written a library to do this for us: https://github.com/mjackson/resolve-pathname

drusellers commented 6 years ago

WANT! Is this on the roadmap to ship in any specific version?

timdorr commented 6 years ago

There is no roadmap. Someone just needs to decide on an API/semantics and code something up.

yordis commented 6 years ago

@timdorr who is gonna be someone? Could you just roll which whatever you feel is the correct path and at least we get the release of this feature rather than no having it?

I am building some distributed interface and this feature is really need it.

svicalifornia commented 6 years ago

I may have some time to work on this in the next few weeks.

timdorr commented 6 years ago

A PR is the very best way to move things forward. We can discuss over code, rather than theory, which is way way faster and better to getting something merged into master.

I'd look through the discussion in here on semantics and then try to build that out. It may require a few PRs to get all the parts into place.