History singletons in non-browser environments

[billyjanitsch]

Suppose I have a component like:

import {browserHistory} from 'react-router'

const LinkButton = props => (
  <Button value={props.to} onPush={browserHistory.push}>
    {props.label}
  </Button>
)

It throws when shallow rendered in a non-browser env, since browserHistory is undefined. It also throws if I access context.router.push, since it just proxies the history passed to Router. This is a big pain for unit testing.

I could wrap history at the top level, but I'd much rather import history directly from the library; isn't that the point of exposing the singleton?

// history.js:
export default isBrowserEnv() ? browserHistory : createMemoryHistory()

// LinkButton.js:
import history from '../history' // ew

I could pass a conditional history to Router, but then I'd always have to rely on context.router which is verbose and prevents me from writing functional components like LinkButton, above.

const history = isBrowserEnv() ? browserHistory : createMemoryHistory()
const router = <Router history={history} routes={routes} />

I could just do this everywhere, but it's verbose, repetitive, and doesn't let me test routing:

<Button value={props.to} onPush={browserHistory ? browserHistory.push : noop}>

One option on the library side is to have the history singleton return more than undefined in non-browser envs. Even just implementing the history interface using noops would at least prevent components throwing. Has this been considered? Is there a better recommended approach?

(I considered this more like a feature request / API discussion than a question, so I posted here rather than StackOverflow, but feel free to redirect me if you disagree!)

[timdorr]

Your presentational LinkButton component should get push passed to it as a prop, otherwise it's not really presentational. The container component above can get that prop from this.context.router.push. This is a better pattern, as your presentational components should not be concerned with application-y things such as routers or state.

[timdorr]

I suggest looking at Dan Abramov's article on the subject: https://medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0#.orydlfr6k

[billyjanitsch]

I'm aware of the presentational/container distinction, but I don't see LinkButton as strictly presentational. It's meant to be like react-router's Link (which doesn't need a push prop), but a button instead of an a.

I guess you're indicating that such a component should always be written as a class rather than a functional component, though.

[taion]

If you have n of these, use an HoC like React-Router-Bootstrap <LinkContainer>.

In either case, use the same patterns we do in <Link> to make it work in tests.

You should not have n components that integrate into the router – wrap these up in HoCs.

[timdorr]

@billyjanitsch Yes, if you need to do things like access context, which functional components don't yet provide, then you'll have to create a class.

[billyjanitsch]

Thanks -- I misinterpreted the purpose of exposing the history singletons in v2. I agree that manipulating history by accessing context is the more robust pattern.

FYI, the second example in this doc is what led me to the approach I described. If you don't actually recommend this, would you like a PR that removes this example, or explains its limitations?

[taion]

I also don't think we really guarantee that you can just split out push like that, instead of calling it via router.push.

[billyjanitsch]

I also don't think we really guarantee that you can just split out push like that, instead of calling it via router.push.

True. PR in #2992.

[gaearon]

Yes, if you need to do things like access context, which functional components don't yet provide, then you'll have to create a class.

You can actually do context with functional components.

function Button(props, context) {
  return <div>{stuff}</div>
}
Button.contextTypes = { stuff: PropTypes.string }

[timdorr]

Ah yes, I forgot about that! Thanks, @gaearon!

[ryanflorence]

For the record, history singletons can be used anywhere in the browser, we added them to make routing super simple and to skip the context dance.

Accessing router from context is "safer" because if you change the history you pass to <Router/> you don't have to update any of your other code. If you switched from the singleton hashHistory to browserHistory, you'd have to update all your usages of it. Even then, you could make a module in your app that exported the one you want to use and get the same safety as accessing router from context.

Feel free to use the singleton histories.

[billyjanitsch]

Even then, you could make a module in your app that exported the one you want to use and get the same safety as accessing router from context.

I suggested this as an option in my first post, and I'm not opposed to it. But if that's the recommended pattern, then I don't understand the advantage of react-router exposing singletons anymore. (If you're going to wrap them anyway, then creating your own singletons in the wrapper is only trivially more work.)