Support additional state that is not in the URL

[insin]

Starting a PR to discuss a feature addition which solved the problems I was having in #792 with form submission and redisplay without having to funnel everything through query.

This adds a new payload parameter to location.push() and location.replace(), the other API methods which ultimately use them, willTransitionTo hooks and as a new property of the state object passed to the router.run() callback.

It also replaces internal use of strings for static locations with a StaticLocation constructor in order to be able to pass a payload object when running on the server.

I've used this in the payload branch of insin/isomorphic-lab with the History location on the client and the new Static location on the server, and in this modified version of the Mega Demo contacts form with the Hash location.

Examples:

• Server-side usage in Express middleware
• Client-side usage in the run() callback
• Isomorphic usage in a component which uses it to pass POST data to a transition and to pass errors (for both server and client) and user input (for redisplay on the server) via a redirect without putting anything in the URL, with the same component acting as the route handler for the entire thing.

Now that I have the basics I needed working, there's other stuff I just fudged my way around or didn't really look at properly, such as:

• Naming, of course. I just called it something generic because I had 2 separate use cases in mind which just so happened to be solved by the same solution.
• History management - should we avoid writing history when flinging this effectively hidden data around? What does the back button behave like and what should it behave like?
• "From" and "to" transition hooks - I fudged this just to get my "to" hooks called when transitioning back to the same component with a payload, without changing the URL. @mjackson mentioned on IRC that this area has a bit of rework coming up in any case.
• Refresh behaviour - should anything special be done here, or should people accept that data will be lost if you use this feature and refresh the resulting page?
• Testing - I just added the new parameter where necessary to get tests passing locally, there are no new test specs for it yet.
• Documentation - this feature is potentially ripe for abuse/misuse by people trying to get to grips with the router, should it be documented in a very use-case/context specific way?

Oh yeah, and is this a feature you want, or is having a built-in way to bypass the URL a bit naughty for a router? 😄

[mjackson]

@insin Thank you for this PR! I haven't had a chance to look very deep yet, but I did push a version of StaticLocation that I've been working on locally for a while now in 193222e, so you may want to rebase and add your payload into that constructor to reduce the scope of this PR a bit. I will look more closely as soon as I get the chance!

[mjackson]

@insin This is really interesting work. It helps me realize a few areas where our API is lacking and/or docs are missing or aren't clear. Thank you :)

I'm still trying to wrap my head around the core concept you're introducing here and use cases you're trying to support. Please correct me if I'm wrong, but it seems like if you were using the HTML5 history API directly your payload arg would serve exactly the same purpose as the state arg to pushState/replaceState. Is that right?

If so, I'd say it's definitely something we'd like to support.

[insin]

@mjackson My initial use case was "POST data doesn't belong in the URL" then it also became "data I have in an async willTransitionTo which is needed for redisplay of an invalid form submission (including the user's submitted input) also doesn't belong on the URL".

Since I'm using routes and route handlers isomorphically, willTransitonTo is the natural place to put request handling-type logic, as is done in the React Router Mega Demo. The only way to get data into that hook (without stepping outside the flow of the router) is via the arguments it receives.

I added an additional parameter to willTransitionTo and worked backwards from there to determine how to provide an argument for it. That's how location.push() and location.replace() (and the methods which ultimately call them) ended up with an extra parameter to facilitate this. That change allowed me to pass an object representing a POST body to be handled by willTransitionTo, which is as hidden from the user as a regular form POST body is.

The only way to get data directly out of a willTransitionTo hook is via aborting the transition and passing something as the reason. The above change inadvertently provided a solution for getting data back out of willTransitionTo without serialising it in the URL, as aborting with a Redirect leads to a location.push() call by default, which now supported an extra parameter for non-URL data, which could then be made part of the state object available when you're rendering the top-level Handler. In a stroke of serendipity, this also meant that a willTransitionTo which needs to pass some data to the next Handler being rendered could be implemented exactly the same for the server and the browser.

Now that you mention it, I can see how something similar could be used for, or as an equivalent to, the state arg to pushState/replaceState but my initial needs were simply: "this stuff doesn't belong on the URL".

[mjackson]

Thanks for the explanation. Since our current API wasn't built to hold state anywhere but the URL path, I think you're probably jumping through some hoops you shouldn't have to if we were to support this properly. :)

Ignoring our current architecture, what I'm hearing from you is there is some state at the application level that you want to be able to tie to a particular invocation of a route that doesn't really belong in the URL. Examples of this extra state include:

• POST data
• error/flash messages
• session data?

Re: the state arg to pushState/replaceState, these kinds of use cases may be what the original authors had in mind when they designed that API. Mind you, I've never actually seen anybody make good use of that argument. In fact, we just put some data in there on the way in but don't do anything with it on the way out. So we're essentially ignoring it. But we could easily take advantage of it here to store this kind of state.

It seems you're on the right track adding your payload arg to the location push/replace API. We could also add a getCurrentState method to the location API so all location objects have a consistent interface to it (instead of just using location.payload).

One big question I have is how we might implement this across our other location objects. StaticLocation and HistoryLocation are pretty straightforward. But HashLocation may need to use some persistent DOM storage ... and maybe RefreshLocation too.

Still trying to wrap my head around this :)

[agundermann]

I think the important thing to decide here is whether the payload should be persisted for history navigation (back/forward), or only used for the next run callback. It seems to me that for POST data, you'd only want to use that once (otherwise you'd probably want to add an extra confirmation dialog for when you go back to that page using back button - like browsers do themselves).

If it should be coupled to history navigation, the feature could make use of #828 by using the history key and managing the payloads within the router (could also be saved to sessionStorage), or it could also directly use the state object. The huge downside, however, is that it won't work reliably with HashLocation or RefreshLocation without messing up URLs (embedding state into query). Maybe I've missed something, but it just doesn't seem possible to tell whether a pop event came from going back one or two pages or even going forward without history.state.

[ryanflorence]

At first I was like "what?" but then if you think about things as "requests" and not "urls" it makes a lot of sense. This would also allow people to send along "context" information that they want access to in their transitions (like flux containers and such).

Also if you think about a browser's back button saying "this will cause a repost", url transitions absolutely have non-url data attached to them. I'm 👍 on supporting a transition context/state arg.

[phuson]

+1

[insin]

Updating the PR's branch again to rename the new object it introduces from payload to data and making data be an additional parameter after callback in willTransitionTo, so it wouldn't break any existing code.

If there's an opportunity to make a breaking change to willTransitionTo in the future, how would you feel about making it take a single object for transition context (context probably isn't a great name because it has the potential to be confused with React's context feature, but naming things is hard 😄)?

willTransitionTo(transition, context[, callback]) {
  // context.params
  // context.query
  // context.data
}

I'd almost be tempted to call it req or request because it'd now have params, query and data (which is effectively body if you're using willTransitionTo for request handling).

[gaearon]

I like putting params and query together in a single object. The current signature for both methods is too long and hard to memorize IMO :-).

[insin]

The need to pass empty params/query objects when you don't need them is ugly, an options object is ugly too in a different way but not having to remember order is a win.

Another one for the "Reasons I wish JavaScript had Python-style kwargs" list.

[framp]

+1

And I think having a proper Request object (with params, query and body) aligned with what we have on node would be awesome and a huge win for isomorphic app development.

I'm working on a fork of react-router which does exactly that (together with a Form element, which is like a Link which passes a req.body). Having it natively in react-router would be awesome.

To reply some of the criticism: I don't think we should store the additional data somewhere, having the same behaviour as a simple POST request (where data is available just once when you issue the request) is probably the thing most people are used to.

[insin]

I have a Form component which does that, too: https://github.com/insin/react-router-form

Just waiting to see how this will fit into the 1.0 API.

[framp]

Oh that's great, looking forward to it

[ryanflorence]

Its in!

Need more docs around it, but here's a tiny bit.

https://rackt.github.io/react-router/tags/v1.0.0-alpha1.html#-transitionto-pathname-query-state-

[insin]

Wunderbar!