Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw helpful error when router reducer not mounted under "router" #175

Merged
merged 4 commits into from Nov 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/ConnectedRouter.js
Expand Up @@ -3,9 +3,11 @@ import PropTypes from 'prop-types'
import { connect } from 'react-redux'
import { Router } from 'react-router'
import { onLocationChanged } from './actions'
import createSelectors from './selectors'

const createConnectedRouter = (structure) => {
const { getIn, toJS } = structure
const { getIn } = structure
const { getRouter, getLocation } = createSelectors(structure)
/*
* ConnectedRouter listens to a history object passed from props.
* When history is changed, it dispatches action to redux store.
Expand All @@ -26,7 +28,7 @@ const createConnectedRouter = (structure) => {
pathname: pathnameInStore,
search: searchInStore,
hash: hashInStore,
} = toJS(getIn(context.store.getState(), ['router', 'location']))
} = getLocation(context.store.getState())
// Extract history's location
const {
pathname: pathnameInHistory,
Expand Down Expand Up @@ -102,8 +104,8 @@ const createConnectedRouter = (structure) => {
}

const mapStateToProps = state => ({
action: getIn(state, ['router', 'action']),
location: getIn(state, ['router', 'location']),
action: getIn(getRouter(state), ['action']),
location: getIn(getRouter(state), ['location']),
})

const mapDispatchToProps = dispatch => ({
Expand Down
17 changes: 14 additions & 3 deletions src/selectors.js
Expand Up @@ -2,8 +2,19 @@ import { matchPath } from "react-router"

const createSelectors = (structure) => {
const { getIn, toJS } = structure
const getLocation = state => toJS(getIn(state, ['router', 'location']))
const getAction = state => toJS(getIn(state, ['router', 'action']))

const isRouter = (value) => value != null &&
typeof value === 'object' &&
getIn(value, ['location']) &&
getIn(value, ['action'])

const getRouter = state => {
mattvague marked this conversation as resolved.
Show resolved Hide resolved
const router = toJS(getIn(state, ['router']))
if (!isRouter(router)) { throw 'Could not find router reducer in state tree, it must be mounted under "router"' }
return router
}
const getLocation = state => toJS(getIn(getRouter(state), ['location']))
const getAction = state => toJS(getIn(getRouter(state), ['action']))

// It only makes sense to recalculate the `matchPath` whenever the pathname
// of the location changes. That's why `createMatchSelector` memoizes
Expand All @@ -27,7 +38,7 @@ const createSelectors = (structure) => {
}
}

return {getLocation, getAction, createMatchSelector}
return {getLocation, getAction, getRouter, createMatchSelector}
}

export default createSelectors
30 changes: 30 additions & 0 deletions test/selectors.test.js
Expand Up @@ -23,6 +23,36 @@ describe("selectors", () => {
store = createStore(reducer)
})

describe("when router not found under 'router' key", () => {
beforeEach(() => {
const reducer = combineReducers({
notTheRouter: connectRouter(history)
})
store = createStore(reducer)
})

it("throws helpful error", () => {
store.dispatch(push('/'))
const state = store.getState()
expect(() => getLocation(state)).toThrowError(/^Could not find router reducer in state tree, it must be mounted under "router"$/)
})
})

describe("when something else found under 'router' key", () => {
beforeEach(() => {
const reducer = combineReducers({
router: () => ({ some: 'thing' })
})
store = createStore(reducer)
})

it("throws helpful error", () => {
store.dispatch(push('/'))
const state = store.getState()
expect(() => getLocation(state)).toThrowError(/^Could not find router reducer in state tree, it must be mounted under "router"$/)
})
})

describe("getLocation", () => {
it("gets the location from the state", () => {
const location = { pathname: "/", hash: '', search: '' }
Expand Down