Skip to content

Commit d60450f

Browse files
committed
Remove subscriptions
1. They are hard to understand 2. They don’t feel like React 3. They were causing *lots of re-rendering on popstate* that we’d have to start doing our own scheduling/batching to get around D: ## But 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}/> )}/> ``` Also, `withRouter` continues to subscribe directly to history if somebody would rather jump into the subscription world instead of passing a prop down. ## What about Redux? We implemented subscriptions so that deep updates work in Redux. 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. ## 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. We’re bowing out of a fight against React where we implement our own scheduling of updates. Also, we still have `withRouter` if people feel like stepping into the ring.
1 parent 8673832 commit d60450f

File tree

4 files changed

+28
-66
lines changed

4 files changed

+28
-66
lines changed

packages/react-router/modules/Route.js

Lines changed: 12 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,13 @@ 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
1612
}
1713

1814
static propTypes = {
@@ -29,37 +25,13 @@ class Route extends React.Component {
2925
location: PropTypes.object
3026
}
3127

32-
static childContextTypes = {
33-
router: PropTypes.object.isRequired
34-
}
35-
36-
getChildContext() {
37-
return {
38-
router: this.router
39-
}
40-
}
41-
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-
})
28+
state = {
29+
match: this.computeMatch(this.props)
5830
}
5931

6032
componentWillReceiveProps(nextProps) {
61-
Object.assign(this.router, {
62-
match: computeMatch(this.router, nextProps)
33+
this.setState({
34+
match: this.computeMatch(nextProps)
6335
})
6436

6537
warning(
@@ -72,13 +44,17 @@ class Route extends React.Component {
7244
)
7345
}
7446

75-
componentWillUnmount() {
76-
this.unlisten()
47+
computeMatch(props) {
48+
const { location, computedMatch, path, exact, strict } = props
49+
const { router } = this.context
50+
const pathname = (location || router.location).pathname
51+
return computedMatch || matchPath(pathname, { path, exact, strict })
7752
}
7853

7954
render() {
8055
const { children, component, render } = this.props
81-
const props = { ...this.router }
56+
const { match } = this.state
57+
const props = { match, ...this.context.router }
8258

8359
return (
8460
component ? ( // component prop gets first priority, only called if there's a match

packages/react-router/modules/Router.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,20 @@ class Router extends React.Component {
2121
}
2222

2323
componentWillMount() {
24-
const { children } = this.props
24+
const { children, history } = this.props
2525

2626
invariant(
2727
children == null || React.Children.count(children) === 1,
2828
'A <Router> may have only one child element'
2929
)
30+
31+
this.unlisten = history.listen(() => {
32+
this.forceUpdate()
33+
})
34+
}
35+
36+
componentWillUnmount() {
37+
this.unlisten()
3038
}
3139

3240
render() {

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 & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import React from 'react'
33
import Router from '../Router'
44
import ReactDOM from 'react-dom'
55

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

@@ -14,7 +16,7 @@ describe('A <Router>', () => {
1416
it('throws an error explaining a Router can only have one child', () => {
1517
expect(() => {
1618
ReactDOM.render(
17-
<Router history={{}}>
19+
<Router history={fakeHistory}>
1820
<p>Foo</p>
1921
<p>Bar</p>
2022
</Router>,
@@ -28,7 +30,7 @@ describe('A <Router>', () => {
2830
it('does not throw an error', () => {
2931
expect(() => {
3032
ReactDOM.render(
31-
<Router history={{}}>
33+
<Router history={fakeHistory}>
3234
<p>Bar</p>
3335
</Router>,
3436
node
@@ -41,7 +43,7 @@ describe('A <Router>', () => {
4143
it('does not throw an error', () => {
4244
expect(() => {
4345
ReactDOM.render(
44-
<Router history={{}} />,
46+
<Router history={fakeHistory} />,
4547
node
4648
)
4749
}).toNotThrow()

0 commit comments

Comments
 (0)