Export action creators in namespace. Add go, goBack, goForward

[timdorr]

You now get to them via the routeActions export.

import { routeActions } from 'redux-simple-router'
routeActions.push('/foo')

This makes binding just the action creators in your connects really easy:

@connect(
  someSelector,
  dispatch => ({ router: bindActionCreators({ routeActions }, dispatch) })
)
class FooBar extends Component {
  handleClick = () => {
    this.props.router.push("/foo")
  }
}

[taion]

Probably add go, goBack, and goForward? They'll just work.

[timdorr]

Done 😄

[timdorr]

Hmm, the functional tests look to be hard to implement, since Warning: Hash history go(n) causes a full page reload in this browser and we're using Firefox (understandable, given @jlongster is a Mozillian 😄). I'll have to add some conditional stuff later.

[jlongster]

Hmm, the functional tests look to be hard to implement, since Warning: Hash history go(n) causes a full page reload in this browser and we're using Firefox (understandable, given @jlongster is a Mozillian 😄). I'll have to add some conditional stuff later.

Can you explain this more? Where does that warning come from? The problem is that Firefox does a full page refresh with go?

[taion]

When using hash history, yes - easy solution is to just add another arg to createTests and set it to false when testing with hash history.

[jlongster]

Ok, I guess I'm confused by the comment about Firefox. Chrome behaves differently?

Also, it's interesting that in React Router 2.0 you can easily change routes from anywhere:

import { browserHistory } from 'react-router'
browserHistory.push(...)

IMHO this removes the need to even really use actions to call the history API. But we probably should keep them around for now.

[taion]

@jlongster

That only works if you're using those singletons. We're a bit undecided right now on whether or not that's an anti-pattern. I think we've mostly tried to avoid that pattern in our examples.

The Chrome/Firefox inconsistency is the Firefox triggers a full page refresh when using history.goBack() or whatever, when using hash history. I don't know why - we have a carveout for this in the history test suite as well. It works fine with browser history (and trivially memory history), though.

[jlongster]

Ah, thanks for explaining that. Unfortunately that behavior is not specified by w3c, which is why they differ.

[timdorr]

Actually, I'm getting a weirder behavior. When I swap out for Chrome, go(-1) (or goBack or goForward) doesn't appear to actually go forward or backwards. The state remains at the last location. No idea what's going on :/

[taion]

@timdorr

It's probably not fully synchronous or something. I know in the history/router tests, we actually put the assertions in the history listener.

What do you think of taking a different approach where we just replace the history with a spy and make sure the right methods get called? I think that's enough coverage-wise. We know the history.listen -> UPDATE_LOCATION code path is fine from the other test cases.

[jlongster]

@timdorr think this is ready to be merged? Basically right now we don't test some of the go functions with a hash history? I'm fine merging this and fixing that later.

[taion]

I don't think we need integration tests there. Using a mock history and asserting that the right calls are made should be sufficient.

[timdorr]

OK, removed those commits for now. It's all good

[billyjanitsch]

Sorry (again) to mention this after shipping v2, but I'm iffy on this change. react-redux's connect already has a shortcut to bind action creators:

// I wish I could do this:
import {push, go} from 'redux-simple-router'
import {myOtherActionCreator} from './myActionCreators'

// these are automatically bound to `dispatch`
export default connect(..., {push, go, myOtherActionCreator}, ...)

To avoid passing extra actions to my component, I now have to manually destructure the routerActions import (which isn't possible with ES6 import syntax -- it requires a const {push, go} = routerActions in every file).

Also, routerActionCreators is technically a more correct name (they're functions that return actions). Some devs choose to omit Creators by convention, but it's weird for an unrelated library to enforce this, and verbose to do import {routerActions as routerActionCreators} from 'redux-simple-router' every time. I believe that's why most Redux libs don't scope their action creator exports.

Would you consider exporting both a scoped object and the individual action creators? (I'm sure there are devs who prefer the scoping.)