Path vs path + query

[simenbrekken]

Right now everywhere in react-router the term "path" is used it refers to the anything after the hostname part of an URI. However RFC3986 specifically states:

The path is terminated by the first question mark ("?") or number sign ("#") character, or by the end of the URI.

I just ran into this today where I was trying to add a query string to an existing URL by means of transitionTo:

this.transitionTo(CurrentPath.getCurrentPath(), null, {dialog: 'account'})

This should produce the following URLs:

• /some/path -> /some/path?dialog=account
• /some/path?dialog=login -> /some/path?dialog=account

But the last URL becomes:

• /some/path/?dialog=login -> /some/path**/dialog=login?dialog=account

This is probably just a regression (it worked before 0.5) but I still think react-router should adhere to the RFC when it comes to words like "path", "query" and "url".

[ryanflorence]

Perhaps some of the authors of this code should level up on the RFC and pay a little bit more attention to the names of things and what they are returning!

I can work on this, or if you'd like, please submit a pull request with whatever changes need to be made.

[simenbrekken]

CurrentPath might be the wrong name for the mixin itself as issues concerning ways to change only parts of the URL keep popping up. Both #378 and #394 could use something like getQuery/setQuery, getHash/setHash, these can also be implemented in <Link> with <Link path="/path" query={{foo: 'bar'}} hash="#foo">.

Thoughts?

[ryanflorence]

the use-case for the CurrentPath mixin came from two things for me personally:

1. parsing it out and adding a className to body
2. embedding a react app inside an iframe and then using postMessage up to the parent with the url so it can keep the url in sync

But its not designed to ever update the path. Maybe it should have some methods that mutate it? Seems nice.

[mjackson]

The RFC does not have a word for path + query. The only kind of "standard" I could find for it was node's URL module, which defines path as:

path: Concatenation of pathname and search.
Example: '/p/a/t/h?query=string'

In general, the node API follows HTML5's URL API, though, admittedly, they differ on this point.

It's also worth noting that node's URL API and window.location both use the word hash (which we also use) instead of fragment (which the spec uses).

@sbrekken Please don't think I didn't think long and hard before I chose these names! :) It's just that there are competing "standards" out there for the same thing.

[mjackson]

Actually, I should correct myself: node's URL API more closely resembles window.location than the HTML5 spec. In any case, I think any of these standards are more relevant than RFC 3986 because they come from actual implementations.

Edit: Also note that window.location has no path property (only pathname and query), which is probably why the node authors chose to use it.

[kharin]

The RFC does not have a word for path + query.

@mjackson, path + query is often referred as URI.

[mjackson]

@kharin Thanks for the reference, tho I'd really like to find one in common use in JavaScript somewhere.

[simenbrekken]

@mjackson I don't really see the benefit of ever letting path be path + query as they are specified separately in the rest of the API (such as transitionTo).

My suggestion is to never include query in path and add additional methods to get/set query.

[mjackson]

@sbrekken There is a substantial benefit from being able to refer to path + query with a single property because the two of them together (with, arguably, a hash property) form the entire state of your app.

If you'd like, I'd be open to using pathname instead of path in the places where we're really only referring to the path portion of the URL without the query. This would conform to node's URL API, as well as window.location, but I would definitely like to keep a way to refer to path + query using a single property going forward.

[simenbrekken]

@mjackson pathname aside from it being referred in other APIs is still confusing to me, is it the name of a specific path? Is it the last segment?

Naming aside, I think my initial example illustrates the ambiguity of the API where transitionTo specifically asks for an URL in three parts, but the corresponding getCurrentPath returns a two of the parameters combined.

[mjackson]

@simenbrekken After a lot of thinking about this I still feel like the way we're currently using the words path, query, and params are all very consistent. transitionTo (and the rest of the Navigation methods) expect 3 arguments: to, params, and query. getPath (was getCurrentPath) returns all three of those arguments in one string.

If you still feel like something needs to be changed, please continue the conversation. But until we have a more concrete example of some inconsistency in the API, I'm going to close.

Thanks for bringing this to our attention! :)

[simenbrekken]

@mjackson I still feel something needs to be changed. Look at #522, having to do:

if (Path.withoutQuery(path) === Path.withoutQuery(prevPath)) {
  return false;
}

Here the query is being stripped from a complete path twice in one statement. And at the same time you have access to state.query.

This leads me to think that the current state API only gives you either the entire car or just the back half. But what if you have some business in the front? That's not currently possible without either manually importing the private Path module in your own project, or using something like nodes url to parse the URL and then feed it back in.

[mjackson]

@simenbrekken One thing we could do to make that statement cleaner would be to provide state.pathname, but I doubt it would be very useful at the app level. Nobody's asked for it yet, probably because we already give you a lot of relevant info about the current URL.

the current state API only gives you either the entire car or just the back half.

The current Router.State API gives you:

• getPath for the entire path + query
• getParams for the portions of the pathname you're interested in
• getQuery for the fully parsed query (no reason to parse anything yourself)
• getRoutes in case you need to know the current routes

For all of the use cases I've seen that API has been sufficient, except for the internal router code you posted above which we could fix by adding state.pathname and getPathname.

[simenbrekken]

@mjackson While I'm still a bit on the fence on naming, I'm more than happy with state.pathname and getPathname.

[anthonator]

I agree with @simenbrekken. There are places where I'd like to construct a path to send to transitionTo within a component. For instance, if we had a list which had the ability to navigate to an edit component we could do something like the following:

this.transitionTo(this.getPath() + "/edit");

This would provide some nice reusability and easier maintenance than passing in a route name as a prop to the component. If you have query params attached to the path you have to either strip them or abandon the approach.

I agree that the API is inconsistent in regard to @simenbrekken's comments. However, even if it wasn't, having the ability to access both halves of the car (if I may borrow some language) would be convenient.