Skip to content

Resolve relative paths #4122

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

Closed
wants to merge 3 commits into from
Closed

Resolve relative paths #4122

wants to merge 3 commits into from

Conversation

pshrmn
Copy link
Contributor

@pshrmn pshrmn commented Oct 31, 2016

I know that @ryanflorence has mentioned some work on relative URLs in other threads, but I thought that I'd make a pull request with a possible working option based on RFC 1808.

I left in comments so that hopefully it should be understandable, but you might also need to reference the RFC 1808 steps. It passes the tests described by 1808, albeit with one modification:

In 1808, anything after the final forward slash in the base url is removed before resolving URLs. However, in React Router, the content after the final forward slash is the base for its children, so it should not be stripped. The tests have been updated to reflect this.

Note: I'm exporting the makeRootRelative function because it was convenient while I was writing this, but the tests for it should just be folded into the createRouterPath tests.

Currently the only tests are the examples from the RFC, so those will probably need to be expanded to ensure correctness, but hopefully this can be a good starting point for addressing the issue (mostly because I dislike writing to={`${pathname}/bar`}).

@@ -8,6 +8,7 @@ describe('HashRouter', () => {
const div = document.createElement('div')

afterEach(() => {
window.location.hash = ''
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to reset this because the 'hashType noslash' tests are run after the 'hashType hashbang' tests, at which point the hash is #!/, which breaks the 'adds a hash link' test because it uses a relative URL and would become #!/foo instead of #foo.

@@ -24,11 +24,58 @@ export const createRouterLocation = (input, parseQueryString, stringifyQuery) =>
}
}

export const createRouterPath = (input, stringifyQuery) => {
return typeof input === 'string' ? input : createPath({
export const createRouterPath = (input, stringifyQuery, base) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make this 3rd arg optional, with a default to ''? Less code changes elsewhere that way.

@timdorr timdorr added this to the v4.0.0 milestone Nov 1, 2016
@pshrmn pshrmn force-pushed the relative-location branch from 2cd33d7 to 22838ed Compare November 5, 2016 06:16
@pshrmn
Copy link
Contributor Author

pshrmn commented Nov 7, 2016

I think that this is at a decent place right now.

The main interaction is done through a resolveLocation function. It takes a location (either a string or a location descriptor) and a base string, which is the absolute pathname to resolve with. If the base string is not provided, it will end up using / as the base.

<Link> and <Redirect> components resolve their to props based on their match parent.

This doesn't try to automatically resolve locations when a user is manually navigating. In order to do that, the resolveLocation function should be exported and the user would just need to resolve based on the pathname of the match that they are in:

import { resolveLocation } from 'react-router'

class MyComponent extends React.Component {
  back(loc) {
    const { match, router } = this.context
    const base = match && match.parent && match.parent.pathname
    router.transitionTo(resolveLocation('..', base))
  }
}

(I'm not actually exporting it in the lastest PR, but that's a simple fix)

@pshrmn pshrmn changed the title [WIP] Use relative paths in Links [WIP] Resolve relative paths Nov 15, 2016
@pshrmn
Copy link
Contributor Author

pshrmn commented Nov 22, 2016

Rebased for the latest version. The new match context is a bit annoying to work with, but everything checks out.

The resolving of relative paths is done using a modified version of RFC
1808. This is done for two reasons:

1) React Router only considers pathnames, so protocol relative paths can
be disregarded.
2) RFC 1808 resolves paths on the last segment with a trailing forward
slash (foo/bar resolves from foo and bar is discarded), but in React
Router the last segment is preserved unless it is an empty string (foo/bar
and foo/bar/ resolve from bar).

<Link> and <Redirect> components have their "to" prop automatically
resolved using their parent match's pathname. If there is no parent
match's pathname, then they are resolving using the root path (an empty
string).

For imperative navigation, the resolveLocation method is exported. The
user is able to generate an absolute location by giving it a location (a
pathname string or a descriptor object) and the base pathname to resolve off of.
@pshrmn pshrmn force-pushed the relative-location branch from ca689af to 32cfdf1 Compare December 6, 2016 20:30
@thevangelist
Copy link

When's this coming in?

@pshrmn pshrmn changed the title [WIP] Resolve relative paths Resolve relative paths Dec 18, 2016
@mjackson
Copy link
Member

I'm afraid this PR is a bit oudated, but I'd like to explain how we're handling relative <Route>s and <Link>s in v4 using the match object.

Relative <Route>s can use the match.path to get the accumulated <Route path> at the position they are rendered:

<Route path="/users/:id" render={({ match }) => (
  <div>
    <Route path={`${match.path}/profile`} component={UserProfile}/>
  </div>
)}/>

Relative <Link>s can use the match.url to get the portion of the URL that matched at the position they are rendered:

<Route path="/users/:id" render={({ match }) => (
  <div>
    <Link to={`${match.url}/profile`}>user profile</Link>
  </div>
)}/>

Thanks for taking the time to contribute :) Please take another look at v4. We're pushing for a beta this week.

@mjackson mjackson closed this Jan 11, 2017
@sgoll
Copy link
Contributor

sgoll commented Jan 11, 2017

@mjackson I think one main use-case for relative paths would be to use .. for going up one level in the hierarchy. Can you do that with relative <Link>s in v4? The last time I checked this was not yet possible.

@mjackson
Copy link
Member

@sgoll Yes, you should be able to use relative <Link to> in v4. It's actually the history library's job to do that, where we're using the resolve-pathname module to resolve relative locations. So, e.g. if you history.push('../foo') it will do the right thing.

You will also probably be interested in the discussion going on in #4332 (comment)

@pshrmn pshrmn deleted the relative-location branch November 12, 2017 05:25
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants