Skip to content

Commit 04fd041

Browse files
committed
Remove subscriptions, withRouter
## Why remove subscriptions? 1. They are hard to understand 2. They fight against React 3. They were causing *lots of cascading re-renders on popstate* that we’d have to start doing our own scheduling/batching to get around D: ## What about shouldComponentUpdate? Pass `location` to the `PureComponent` (or whatever). If a component as a Route or Switch in it then `location` needs to be a part of the sCU diffing. ```jsx <Route render={({ location }) => ( <SomePureComponent location={location}/> )}/> ``` We implemented subscriptions so that deep updates would work in Redux out of the box (no react-router-redux needed). But, Redux users want more than just that. They want to: 1. get deep updates that survive redux’s automatic sCU 2. read routing state from the store, not components 3. navigate with dispatch 4. sync the url and store so the devtools time-travel/upload debugging tools work Subscriptions only solved (1). To get the rest requires the exact same touch points whether we do subscriptions or not. ## Why remove withRouter? It has the same cascading render problem (because it used subscriptions), we don’t want to deal with that. `withRouter` has been about getting access to the imperative router API. It can easily be added to app manually in two lines: ``` const withRouter = (Comp) => (props) => <Route render={(router) => <Comp {…props} {…router}/> ``` ## In Summary Since the Redux integration story is identical whether we use subscriptions or not, and normal sCU can be solved by just passing the `location` to the optimized components, there’s no reason we need to keep using subscriptions and fighting against React.
1 parent 7ba62ea commit 04fd041

File tree

7 files changed

+36
-250
lines changed

7 files changed

+36
-250
lines changed

packages/react-router/modules/Route.js

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@ import React, { PropTypes } from 'react'
22
import warning from 'warning'
33
import matchPath from './matchPath'
44

5-
const computeMatch = (router, { location, computedMatch, path, exact, strict }) =>
6-
computedMatch || matchPath((location || router.location).pathname, { path, exact, strict })
75

86
/**
97
* The public API for matching a single path and rendering.
108
*/
119
class Route extends React.Component {
1210
static contextTypes = {
13-
router: PropTypes.shape({
14-
listen: PropTypes.func.isRequired
15-
}).isRequired
11+
router: PropTypes.object.isRequired
12+
}
13+
14+
static childContextTypes = {
15+
router: PropTypes.object.isRequired
1616
}
1717

1818
static propTypes = {
@@ -29,37 +29,22 @@ class Route extends React.Component {
2929
location: PropTypes.object
3030
}
3131

32-
static childContextTypes = {
33-
router: PropTypes.object.isRequired
32+
state = {
33+
match: this.computeMatch(this.props)
3434
}
3535

3636
getChildContext() {
3737
return {
38-
router: this.router
38+
router: {
39+
...this.context.router,
40+
match: this.state.match
41+
}
3942
}
4043
}
4144

42-
componentWillMount() {
43-
const parentRouter = this.context.router
44-
45-
this.router = {
46-
...parentRouter,
47-
match: computeMatch(parentRouter, this.props)
48-
}
49-
50-
// Start listening here so we can <Redirect> on the initial render.
51-
this.unlisten = parentRouter.listen(() => {
52-
Object.assign(this.router, parentRouter, {
53-
match: computeMatch(parentRouter, this.props)
54-
})
55-
56-
this.forceUpdate()
57-
})
58-
}
59-
6045
componentWillReceiveProps(nextProps) {
61-
Object.assign(this.router, {
62-
match: computeMatch(this.router, nextProps)
46+
this.setState({
47+
match: this.computeMatch(nextProps)
6348
})
6449

6550
warning(
@@ -72,19 +57,23 @@ class Route extends React.Component {
7257
)
7358
}
7459

75-
componentWillUnmount() {
76-
this.unlisten()
60+
computeMatch(props) {
61+
const { location, computedMatch, path, exact, strict } = props
62+
const { router } = this.context
63+
const pathname = (location || router.location).pathname
64+
return computedMatch || matchPath(pathname, { path, exact, strict })
7765
}
7866

7967
render() {
8068
const { children, component, render } = this.props
81-
const props = { ...this.router }
69+
const { match } = this.state
70+
const props = { ...this.context.router, match }
8271

8372
return (
8473
component ? ( // component prop gets first priority, only called if there's a match
85-
props.match ? React.createElement(component, props) : null
74+
match ? React.createElement(component, props) : null
8675
) : render ? ( // render prop is next, only called if there's a match
87-
props.match ? render(props) : null
76+
match ? render(props) : null
8877
) : children ? ( // children come last, always called
8978
typeof children === 'function' ? (
9079
children(props)

packages/react-router/modules/Router.js

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class Router extends React.Component {
1616

1717
getChildContext() {
1818
return {
19-
router: this.router
19+
router: this.props.history
2020
}
2121
}
2222

@@ -28,24 +28,15 @@ class Router extends React.Component {
2828
'A <Router> may have only one child element'
2929
)
3030

31-
this.router = {
32-
...history,
33-
match: {
34-
path: '/',
35-
url: '/',
36-
params: {},
37-
isExact: history.location.pathname === '/'
38-
}
39-
}
40-
41-
history.listen(() => {
42-
Object.assign(this.router, history)
43-
Object.assign(this.router.match, {
44-
isExact: history.location.pathname === '/'
45-
})
31+
this.unlisten = history.listen(() => {
32+
this.forceUpdate()
4633
})
4734
}
4835

36+
componentWillUnmount() {
37+
this.unlisten()
38+
}
39+
4940
render() {
5041
const { children } = this.props
5142
return children ? React.Children.only(children) : null

packages/react-router/modules/Switch.js

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,14 @@ import matchPath from './matchPath'
77
*/
88
class Switch extends React.Component {
99
static contextTypes = {
10-
router: PropTypes.shape({
11-
listen: PropTypes.func.isRequired
12-
}).isRequired
10+
router: PropTypes.object.isRequired
1311
}
1412

1513
static propTypes = {
1614
children: PropTypes.node,
1715
location: PropTypes.object
1816
}
1917

20-
state = {
21-
location: this.props.location || this.context.router.location
22-
}
23-
24-
componentWillMount() {
25-
if (!this.props.location) {
26-
const { router } = this.context
27-
28-
// Start listening here so we can <Redirect> on the initial render.
29-
this.unlisten = router.listen(() => {
30-
this.setState({
31-
location: router.location
32-
})
33-
})
34-
}
35-
}
36-
3718
componentWillReceiveProps(nextProps) {
3819
warning(
3920
!(nextProps.location && !this.props.location),
@@ -46,14 +27,9 @@ class Switch extends React.Component {
4627
)
4728
}
4829

49-
componentWillUnmount() {
50-
if (this.unlisten)
51-
this.unlisten()
52-
}
53-
5430
render() {
5531
const { children } = this.props
56-
const { location } = this.state
32+
const location = this.props.location || this.context.router.location
5733

5834
let match, child
5935
React.Children.forEach(children, element => {

packages/react-router/modules/__tests__/Router-test.js

Lines changed: 5 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import Router from '../Router'
44
import ReactDOM from 'react-dom'
55
import createHistory from 'history/createMemoryHistory'
66

7+
const fakeHistory = { listen: () => () => {} }
8+
79
describe('A <Router>', () => {
810
const node = document.createElement('div')
911

@@ -15,7 +17,7 @@ describe('A <Router>', () => {
1517
it('throws an error explaining a Router can only have one child', () => {
1618
expect(() => {
1719
ReactDOM.render(
18-
<Router history={createHistory()}>
20+
<Router history={fakeHistory}>
1921
<p>Foo</p>
2022
<p>Bar</p>
2123
</Router>,
@@ -29,7 +31,7 @@ describe('A <Router>', () => {
2931
it('does not throw an error', () => {
3032
expect(() => {
3133
ReactDOM.render(
32-
<Router history={createHistory()}>
34+
<Router history={fakeHistory}>
3335
<p>Bar</p>
3436
</Router>,
3537
node
@@ -42,89 +44,10 @@ describe('A <Router>', () => {
4244
it('does not throw an error', () => {
4345
expect(() => {
4446
ReactDOM.render(
45-
<Router history={createHistory()} />,
47+
<Router history={fakeHistory} />,
4648
node
4749
)
4850
}).toNotThrow()
4951
})
5052
})
51-
52-
describe('context.router', () => {
53-
let rootContext
54-
const ContextChecker = (props, context) => {
55-
rootContext = context.router
56-
return null
57-
}
58-
ContextChecker.contextTypes = { router: React.PropTypes.object }
59-
60-
afterEach(() => {
61-
rootContext = undefined
62-
})
63-
64-
it('sets a root match', () => {
65-
const history = createHistory({ initialEntries: ['/'] })
66-
ReactDOM.render(
67-
<Router history={history}>
68-
<ContextChecker />
69-
</Router>,
70-
node
71-
)
72-
expect(rootContext.match).toEqual({
73-
path: '/',
74-
url: '/',
75-
params: {},
76-
isExact: true
77-
})
78-
})
79-
80-
it('spreads the history object\'s properties', () => {
81-
const history = createHistory()
82-
ReactDOM.render(
83-
<Router history={history}>
84-
<ContextChecker />
85-
</Router>,
86-
node
87-
)
88-
89-
Object.keys(history).forEach(key => {
90-
expect(rootContext[key]).toEqual(history[key])
91-
})
92-
})
93-
94-
it('updates history properties upon navigation', () => {
95-
const history = createHistory()
96-
ReactDOM.render(
97-
<Router history={history}>
98-
<ContextChecker />
99-
</Router>,
100-
node
101-
)
102-
expect(rootContext.length).toBe(1)
103-
104-
const newLocation = { pathname: '/new' }
105-
history.push(newLocation)
106-
107-
Object.keys(newLocation).forEach(key => {
108-
expect(rootContext.location[key]).toEqual(newLocation[key])
109-
})
110-
expect(rootContext.action).toBe('PUSH')
111-
expect(rootContext.length).toBe(2)
112-
})
113-
114-
it('updates match.isExact upon navigation', () => {
115-
const history = createHistory({ initialEntries: ['/'] })
116-
ReactDOM.render(
117-
<Router history={history}>
118-
<ContextChecker />
119-
</Router>,
120-
node
121-
)
122-
expect(rootContext.match.isExact).toBe(true)
123-
124-
const newLocation = { pathname: '/new' }
125-
history.push(newLocation)
126-
127-
expect(rootContext.match.isExact).toBe(false)
128-
})
129-
})
13053
})

packages/react-router/modules/__tests__/withRouter-test.js

Lines changed: 0 additions & 57 deletions
This file was deleted.

packages/react-router/modules/index.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,3 @@ export Router from './Router'
66
export StaticRouter from './StaticRouter'
77
export Switch from './Switch'
88
export matchPath from './matchPath'
9-
export withRouter from './withRouter'

0 commit comments

Comments
 (0)