Skip to content

Commit

Permalink
Refactor ConnectedRouter to listen to history directly
Browse files Browse the repository at this point in the history
  • Loading branch information
inxilpro committed Mar 15, 2017
1 parent 6ae5f65 commit d3e3a48
Showing 1 changed file with 48 additions and 15 deletions.
63 changes: 48 additions & 15 deletions packages/react-router-redux/modules/ConnectedRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ class ConnectedRouter extends Component {
store: PropTypes.object
}

state = {
history: null,
location: {
key: null
}
}

unlisten = null

componentWillMount() {
const { children } = this.props

Expand All @@ -24,22 +33,46 @@ class ConnectedRouter extends Component {
)
}

componentDidMount() {
this.handleProps(this.props)
this.handleLocation(this.props.history.location)
}

componentDidUpdate() {
this.handleProps(this.props)
}

componentWillUnmount() {
if (this.unlisten) {
this.unlisten()
}
}

handleProps(props) {
const { history } = props
if (history !== this.state.history) {
if (this.unlisten) {
this.unlisten()
}
this.setState({ history, location: history.location })
this.unlisten = history.listen(this.handleLocation.bind(this))
}
}

handleLocation(nextLocation) {
const store = this.props.store || this.context.store
if (nextLocation.key !== this.state.location.key) {

This comment has been minimized.

Copy link
@GuillaumeCisco

GuillaumeCisco Mar 16, 2017

Why is this test present?
It won't work with createHashHistory (mainly used for development purposes)

This comment has been minimized.

Copy link
@inxilpro

inxilpro Mar 16, 2017

Author Owner

Oh, fair point. Just trying to avoid dispatching if unnecessary, but that's really the job of the history lib, so it can be removed.

This comment has been minimized.

Copy link
@GuillaumeCisco

GuillaumeCisco Mar 17, 2017

Yep, I understood this point after posting my comment :)
I'm using this code in production and it works great, with a little modification. Hope it will be temporary.

if (nextLocation.key !== this.state.location.key ||
            nextLocation.pathname !== this.state.location.pathname ||
            nextLocation.search !== this.state.location.search ||
            nextLocation.hash !== this.state.location.hash) {
            this.setState({
                location: nextLocation
            }, () => store.dispatch({
                type: LOCATION_CHANGE,
                payload: nextLocation
            }));
        }

The equality check should be on the object itself with a cache registry manager. Or using immutable for comparing on the hash of the object instead of the fields.
For now, as all of it is experimental, I'm waiting :)

this.setState({
location: nextLocation
}, () => store.dispatch({
type: LOCATION_CHANGE,
payload: nextLocation
}));
}
}

render() {
const { store:propsStore, history, children, ...props } = this.props
let store = propsStore || this.context.store

return (
<Router {...props} history={history}>
<Route render={({ location }) => {
store.dispatch({
type: LOCATION_CHANGE,
payload: location
})

return children ? React.Children.only(children) : null
}}/>
</Router>
)
return <Router {...this.props} />;
}
}

Expand Down

0 comments on commit d3e3a48

Please sign in to comment.