should history be router API or implementation detail?

[ryanflorence]

Right now this is kinda weird:

const myOwnHistory = createHistory()
render(<Router history={myOwnHistory}/>)

// in a route component
this.context.history === myOwnHistory // false

So what?

1. Some apps (redux/flux especially) like to have their own app history so they can navigate around in actions.
2. Router and match wrap the history prop with useQueries and useRoutes. In order to transition in an action with myOwnHistory you have to useQueries yourself, causing it to be wrapped twice :(
3. If you listen to your history, and then hand it to the router, it enhances with useRoutes and now the listeners fire in unpredictable orders

What if history wasn't primary API?

All most apps need from history is to push or replace. What if we just don't expose history at all, and just expose some functions that wrap history to do what you need inside of components, like maybe replaceLocation and pushLocation.

What would it look like?

import { Router } from 'react-router'
render(<Router history="browser" routes={routes}/>, document.getElementById('app'))

// in a component
this.context.router.pushLocation('/users/123')

What about redux and friends?

For people who want their own history (for redux actions, etc.) we just use whatever history they give us without wrapping it, in most cases would be as easy as import { browserHistory } from 'react-router' .

import { browserHistory, Router } from 'react-router'
render(<Router history={browserHistory} routes={routes}/>, document.getElementById('app'))

// in an action
import { browserHistory } from 'react-router'
browserHistory.push(...)

Sales pitch

1. We don't have to document history in router, we can move useRoutes out into its own package and document it there.
2. useRoutes is useful for non-react apps, makes sense to move it out
3. People who want to use their own history, or navigate around outside of components, can learn about the history api from the documentation of those projects
4. Router documentation becomes much simpler.

I might work up a PR to hit with a stick.

[ryanflorence]

Also, @mjackson and I talked about this while traveling around in London and he's on board (with this idea, he got off the train yesterday)

[taion]

I was trying to get at this a bit with #2502.

There's a slight issue with the API above, in that the routes prop to the <Router> is out of place. If the user wants to pass in a wrapped history, then schematically the API might look like:

<Router history={useRoutes(useQueries((createHistory))({routes})} />

This is what I meant w/r/t the "90%" API, in that I feel like most users would prefer to do

<Router history={createBrowserHistory()} routes={routes} />

Like you pointed out, this means that <Router> has to wrap the history... but it's just a nicer-looking API ):

What I meant to get at with #2502 is that, in addition to just being a cleaner API, having a listenRoutes on history means that <Router> can decide whether or not to do the wrapping.

That's a bit "ick", but it lets us preserve the API where the 90% use case can just pass in routes to the <Router>, and not have to think about useRoutes. IMO this API is more beginner-friendly.

[taion]

tl;dr useRoutes makes the routes a property of the history, not the <Router>, and I like giving users the <Router history={history} routes={routes} /> API for simple use cases, rather than making users think about useRoutes.

[taion]

BTW, the useQueries is actually worse than stated - since useQueries appends the queries to search rather than replacing search, if you do double-wrap, you'll get your queries duplicated in the URL. Ick.

I hack around this for remix-run/history#141 but it's a bit messy and we should make a breaking change there.

[ryanflorence]

There's a slight issue with the API above, in that the routes prop to the is out of place.

Oh, yep. Seems like we can see they sent a string "browser" or "hash" and then wrap, otherwise we leave it all alone.

So 90%:

import { Router } from 'react-router'
// we know to wrap because they sent a string (an old, beloved API we once had)
render(<Router history="browser" routes={routes}/>, document.getElementById('app'))

10% - folks who want their own history

const history = useRoutes(useQueries((createHistory))({routes})
<Router history={history} />

[taion]

That's one option. I don't think it's entirely ideal, because it restricts the set of "raw" histories that can be used, and means that we have to pull all of them into the React Router package (I'm unhappy enough as-is that we always pull in createHashHistory).

What I'd like to see is that if we merge #2580, we can just check for whether history.listenRoutes exists. If it exists, then we use that history-as is. Otherwise, we wrap it. It's a little harder to explain to users, but it should really only matter for that 10% case, and I'm okay with the tradeoff to make it a little harder to understand.

[ryanflorence]

Ah, yeah this is kinda hairy, let me think on this a bit.

[taion]

My thought was to expose something like

export default function wrapHistory({
  history = createHashHistory(), routes, children
}) {
  if (history.listenRoutes) {
    return history
  }

  return useRoutes(() => history)({
    routes: createRoutes(routes || children),
  })
}

It's not beautiful, but the nice thing is that e.g. redux-simple-router can use this function to make sure it has a routing history, then call history.listenRoutes on that object.

EDIT: Code style

[ryanflorence]

Here's another idea

<Router history={someHistory}/>

If you didn't provide props.children, and didn't provide props.routes, we can assume you know what you're doing and have already wrapped your history with useRoutes and we'll just use it.

[taion]

We can do that. My thought with something like wrapHistory above is that this could be useful to writers of integrations. Consider a hypothetical Redux integration - I'd think it should look like e.g.

<ReduxRouter history={history} store={store} routes={routes} />

If we expose that sort of wrapHistory thing above (although this doesn't require the listenRoutes thing, though I still think that's nice to have), it could let integrations that just want to wrap the "routing history" to call into the same function for ensuring that they have a routing history, while offering the same API to users on <ReduxRouter>, <AltRouter>, and all the rest.

[taion]

This is a bit orthogonal, though, since we can still do #2614 (comment) and #2614 (comment). It just depends on whether we think listenRoutes makes sense, I guess.

[timdorr]

Perhaps instead, you could introduce another prop to make this very explicit:

<Router createHistory={createBrowserHistory} routes={routes} />
// useRoutes(useQueries(this.props.createHistory()))({ routes }) is inside of Router

// Otherwise, assume the history is ready to go.
const reduxHistory = useRoutes(useQueries((createReduxHistory(store)))({routes})
<Router history={reduxHistory} />

[taion]

👍: Getting rid of () => history would be nice
👎: It still feels a bit magical, and it's a breaking API change

I really like checking for history.listenRoutes is a nice way to go, if we like listenRoutes - I personally like it because I think it's a good change to make given the change to the signature of the callback to listen that useRoutes makes.

But enough shilling for my own idea (:

[ryanflorence]

Been thinking about this off and on the last couple days, and discussing with @mjackson.

What if coupling route config to history via useRoutes is the wrong approach?

1. It seems strange that if the user clicks the back button twice really fast an application that cares about those changes (outside of rendering) can potentially miss one of the history states there (because useRoutes is async and doesn't send a new location for that intermediary location)
2. It's weird to change the signature of listen
3. We're tempted to add a new method listenRoutes because listen before and after useRoutes fires at different times
4. We can't export a browserHistory singleton for people to use to control the browser history
5. In order for Redux to have a history it can listen to, we're expecting them to wrap it up in a bunch of stuff, and even then, its unclear how to then handle the history they provide us in Router (again, tempting us to add API listenRoutes).

We already have match for server side rendering. Maybe that's the right separation.

1. Router takes whatever history it gets and does little more than listen to it and call match with the location
2. RoutingContext no longer exports history to context, but instead exports isActive, pushLocation, and replaceLocation (and to decrease the size of our context footprint, hangs everything it exports to context on a router object)

Now, I think we have cleaner solutions to all of those issues w/o new API, maybe even less.

[mjackson]

Ya, agree with @ryanflorence. useRoutes feels like the culprit. Still thinking about this a lot.

[mjackson]

What if we just export a match function? That's the main responsibility of useRoutes.

Our API could just be boiled down to:

import { render } from 'react-dom'
import { match, RoutingContext } from 'react-router'

history.listen(location => {
  match(location, routes, (error, state) => {
    render(<RoutingContext {...state} />, node)
  })
})

Pro: Things like isActive and push/replace would live in <RoutingContext> so we don't have to use the history API all over the place.

Con: More boilerplate for common use cases, but maybe we could wrap it up in <Router>...

[timdorr]

I think you've hit the nail on the head. What's the point of having a separate history library if you're going to manipulate it so heavily in useRoutes? I imagine if history had already existed before Router, then we would have just merged it into this repo eventually.

We should be able to use history's APIs without any monkey patching. If not, we need to enhance history, which benefits everyone.

[timdorr]

By "enhance history", I mean develop features and code in the history library, not enhance it in Router's code.

[taion]

I'm 👍👍👍 on the core idea here. useRoutes doesn't fit as a "history enhancer", because listeners to routes want something different from listeners to history. For most users of the router, history.listen is an implementation detail.

That said, taking an inventory of extensions, I think we can actually keep the API almost in its entirety. The reason we can do this is because most of our extensions don't need to listen to the routing history, and it's entirely an accident that they do.

To take a quick inventory:

AsyncProps, react-router-relay

Easy case, neither care about history at all, they just receive route matches via custom routing context.

redux-simple-router, alt-router

cc @jlongster, @goatslacker

These don't need to use history.listen to get routing state! Why not just do componentWillReceiveProps in a custom routing context? Or, for a more lightweight way:

const routes = (
  <Route component={connectToStore(store)}>
    {actualRoutes}
  </Route>
)

That should be good enough for redux-simple-router to work as intended.

redux-router

I actually have no clue how redux-router works.

scroll-behavior

The current API has scroll-behavior work as a history enhancer. It's actually similar enough to the redux-simple-router and alt-router cases above that I'd probably want to re-write scroll-behavior to work in the same way - probably just lean toward making it an abstract route like in the above code snippet, which also then might let us implement @ryanflorence's idea of managing scroll behavior for individual scroll-able components.

Future native router

One main concern here is integrating with <Navigator> anyway to take advantage of its support for animations, gestures, &c., so we'd need to do changes to internals anyway most likely to get that to work, so I'm not worried there.

[taion]

BTW, I think the best way to do this would be to have something like:

<Router history={history} matcher={matcher} />

The history is exactly what it is now. The matcher would be the new thing, but it'd take a default value that gives the current behavior.

The idea would be that the matcher would subscribe to the history, then the <Router> would subscribe to the matcher. Basically the matcher would be the current useRoutes enhancer, except it wouldn't wrap the history.

This lets us keep the exact same API for users - I don't think there's a good reason to wrap history.push and history.replace because they should IMO just be straight pass-throughs. The user should never have to think about the matcher at all - it's just a (replaceable) implementation detail of the router itself - none of our API points really touch on it, except:

• Checking for isActive
• Setting up route "transition out" listeners

I don't know how to handle those two cases above, but I feel like the idea of making this.context.history just be the history supplied to <Router> and keeping the API there gives us something quite nice and clean.

[taion]

To flesh out my proposal a bit more, the user-facing API would look something like:

static contextTypes = {history, router, location}

Then you can do:

this.context.history.push(location)
this.context.history.replace(location)

const isActive = this.context.router.isActive(locationSpec)
const unlisten = this.context.router.listenBeforeLeavingRoute(route, hook)

// A form where we explicitly specify the location to compare against would let
// us clean up the animation example, maybe enough to drop the inscrutable
// <StaticContainer> there.
const isActive = this.context.matcher.isActive(
  locationSpec, this.context.location
)

It's a slightly larger context surface area, but it has the nice property of exactly preserving the history API for transitions, which seems nice to me.

[taion]

One more wrinkle though.

It'd be nice to be able to change async PUSH transitions into REPLACE transitions if they happen before the async route and component data for the previous transition complete.

I believe we don't have this right now. My understanding is that currently history synchronously processes the transition and updates the URL, at which point it's done, and the router goes on its merry way doing its route matching thing. (This is actually different from a native transition, where e.g. the URL doesn't update until the async stuff is complete).

I don't think we can fully mimic this behavior without the router connecting deeply into the history, because we'd need to suppress PUSH transitions on history from finishing until after the async loads complete.

I can't really think of a good way to handle this at all, but I feel like it might require some coupling between the history and the async routing, or else the explicit introduction of async handling in the history.

[taion]

Eh, I take that back, maybe we just need to do the routing from history.listenBefore rather than history.listen. Oops.

[kidwm]

If this could make it feasible using react-router without history, I'm glad to see it.

[taion]

We need history regardless - it's just a question of the contract between <Router> and history and users of both.

[ryanflorence]

see #2646