Skip to content

Commit 227a2ed

Browse files
committed
Change the way Match/Miss integrate
The only guarantee from React regarding the lifecycle of ancestors and descendants is that cDM of descendants will be called before cDM of ancestors. Also, in the future (React Fibers), cWM may get called but the component may never get mounted, and there won’t be a lifecycle hook to know the component’s rendering was cancelled (facebook/react#7671). So, instead, we add matches in cDM instead of cWM, and then subscribe to the MatchProvider. When the ancestor MatchProvider mounts, it notifies the misses so they know whether or not they should render. This won’t work on the server, but that’s next!
1 parent ccfc062 commit 227a2ed

File tree

4 files changed

+94
-36
lines changed

4 files changed

+94
-36
lines changed

modules/Match.js

+10-8
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,31 @@ class RegisterMatch extends React.Component {
1212
match: PropTypes.object
1313
}
1414

15-
componentWillMount() {
15+
componentDidMount() {
1616
const { match:matchContext } = this.context
1717
const { match } = this.props
1818

19-
if (match && matchContext)
19+
if (match && matchContext) {
2020
matchContext.addMatch(match)
21+
}
2122
}
2223

23-
componentWillReceiveProps(nextProps) {
24+
componentDidUpdate(prevProps) {
2425
const { match } = this.context
2526

2627
if (match) {
27-
if (nextProps.match && !this.props.match) {
28-
match.addMatch(nextProps.match)
29-
} else if (!nextProps.match && this.props.match) {
30-
match.removeMatch(this.props.match)
28+
if (prevProps.match && !this.props.match) {
29+
match.removeMatch(prevProps.match)
30+
} else if (!prevProps.match && this.props.match) {
31+
match.addMatch(this.props.match)
3132
}
3233
}
3334
}
3435

3536
componentWillUnmount() {
36-
if (this.props.match)
37+
if (this.props.match) {
3738
this.context.match.removeMatch(this.props.match)
39+
}
3840
}
3941

4042
render() {

modules/MatchProvider.js

+31-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ class MatchProvider extends React.Component {
1919
// React doesn't support a parent calling `setState` from an descendant's
2020
// componentWillMount, so we use an instance property to track matches
2121
this.matches = []
22+
this.subscribers = []
23+
this.hasMatches = null // use null for initial value
2224
}
2325

2426
addMatch = match => {
@@ -38,7 +40,35 @@ class MatchProvider extends React.Component {
3840
parent: this.parent,
3941
matches: this.matches,
4042

41-
matchFound: () => this.matches.length > 0
43+
subscribe: (fn) => {
44+
this.subscribers.push(fn)
45+
return () => {
46+
this.subscribers.splice(this.subscribers.indexOf(fn), 1)
47+
}
48+
}
49+
50+
}
51+
}
52+
}
53+
54+
componentDidUpdate() {
55+
this.notifySubscribers()
56+
}
57+
58+
componentDidMount() {
59+
this.notifySubscribers()
60+
}
61+
62+
notifySubscribers() {
63+
// React's contract is that cDM of descendants is called before cDM of
64+
// ancestors, so here we can safely check if we found a match
65+
if (this.subscribers.length) {
66+
const hadMatches = this.hasMatches
67+
this.hasMatches = this.matches.length !== 0
68+
// optimization, don't notify if nothing changed initial will be null, so
69+
// we can get initial render correct
70+
if (this.hasMatches !== hadMatches) {
71+
this.subscribers.forEach(fn => fn(this.hasMatches))
4272
}
4373
}
4474
}

modules/Miss.js

+25-11
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,38 @@ class Miss extends React.Component {
1010
}
1111

1212
static contextTypes = {
13-
match: PropTypes.object.isRequired,
14-
location: PropTypes.object.isRequired,
13+
match: PropTypes.object,
14+
location: PropTypes.object,
1515
serverRouter: PropTypes.object
1616
}
1717

18+
state = {
19+
noMatchesInContext: false
20+
}
21+
22+
componentWillMount() {
23+
// ignore if rendered out of context (probably for unit tests)
24+
if (this.context.match) {
25+
this.unsubscribe = this.context.match.subscribe((matchesFound) => {
26+
this.setState({
27+
noMatchesInContext: !matchesFound
28+
})
29+
})
30+
}
31+
}
32+
33+
componentWillUnmount() {
34+
if (this.unsubscribe) {
35+
this.unsubscribe()
36+
}
37+
}
38+
1839
render() {
1940
const { render, component:Component } = this.props
20-
const { match, serverRouter } = this.context
41+
const { noMatchesInContext } = this.state
2142
const { location:locationProp } = this.props
2243
const location = locationProp || this.context.location
23-
if (!match) {
24-
// don't render if out of context (probably a unit test)
25-
return null
26-
} else if (!match.matchFound()) {
27-
// side-effect in render, only happens on the server
28-
// and calling it multiple times should be fine
29-
if (serverRouter)
30-
serverRouter.onMiss(location)
44+
if (noMatchesInContext) {
3145
return (
3246
render ? (
3347
render({ location })

modules/__tests__/Miss-test.js

+28-16
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,33 @@
11
import expect from 'expect'
22
import React from 'react'
33
import Miss from '../Miss'
4-
import { renderToString } from 'react-dom/server'
4+
import { render, unmountComponentAtNode } from 'react-dom'
55
import MatchProvider from '../MatchProvider'
66

77
describe('Miss', () => {
88
const TEXT = 'TEXT'
9+
const loc = { pathname: '/', search: '', hash: '', state: TEXT }
910

10-
it('renders a Component prop', () => {
11+
it('renders a Component prop', (done) => {
12+
const div = document.createElement('div')
1113
const Page = () => <div>{TEXT}</div>
12-
const html = renderToString(
14+
render((
1315
<MatchProvider>
1416
<Miss
15-
location={{}}
17+
location={loc}
1618
component={Page}
1719
/>
1820
</MatchProvider>
19-
)
20-
expect(html).toContain(TEXT)
21+
), div, () => {
22+
expect(div.innerHTML).toContain(TEXT)
23+
unmountComponentAtNode(div)
24+
done()
25+
})
2126
})
2227

23-
it('renders a render prop passes a location', () => {
24-
const loc = { state: TEXT }
25-
const html = renderToString(
28+
it('renders a render prop passes a location', (done) => {
29+
const div = document.createElement('div')
30+
render((
2631
<MatchProvider>
2732
<Miss
2833
location={loc}
@@ -31,18 +36,25 @@ describe('Miss', () => {
3136
)}
3237
/>
3338
</MatchProvider>
34-
)
35-
expect(html).toContain(TEXT)
39+
), div, () => {
40+
expect(div.innerHTML).toContain(TEXT)
41+
unmountComponentAtNode(div)
42+
done()
43+
})
3644
})
3745

38-
it('renders null when out of context', () => {
46+
it('renders null when out of context', (done) => {
47+
const div = document.createElement('div')
3948
const Page = () => <div>{TEXT}</div>
40-
const html = renderToString(
49+
render((
4150
<Miss
42-
location={{}}
51+
location={loc}
4352
component={Page}
4453
/>
45-
)
46-
expect(html).toNotContain(TEXT)
54+
), div, () => {
55+
expect(div.innerHTML).toNotContain(TEXT)
56+
unmountComponentAtNode(div)
57+
done()
58+
})
4759
})
4860
})

0 commit comments

Comments
 (0)