Skip to content

Commit

Permalink
Persist child listeners through hot reload (reduxjs#715)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsgkirkby authored and josepot committed Sep 21, 2018
1 parent 1e8f673 commit 46d8b25
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 17 deletions.
17 changes: 15 additions & 2 deletions src/components/connectAdvanced.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,22 @@ export default function connectAdvanced(
this.version = version
this.initSelector()

if (this.subscription) this.subscription.tryUnsubscribe()
// If any connected descendants don't hot reload (and resubscribe in the process), their
// listeners will be lost when we unsubscribe. Unfortunately, by copying over all
// listeners, this does mean that the old versions of connected descendants will still be
// notified of state changes; however, their onStateChange function is a no-op so this
// isn't a huge deal.
let oldListeners = [];

if (this.subscription) {
oldListeners = this.subscription.listeners.get()
this.subscription.tryUnsubscribe()
}
this.initSubscription()
if (shouldHandleStateChanges) this.subscription.trySubscribe()
if (shouldHandleStateChanges) {
this.subscription.trySubscribe()
oldListeners.forEach(listener => this.subscription.listeners.subscribe(listener))
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/utils/Subscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ function createListenerCollection() {
}
},

get() {
return next
},

subscribe(listener) {
let isSubscribed = true
if (next === current) next = current.slice()
Expand Down
91 changes: 76 additions & 15 deletions test/components/connect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ describe('React', () => {
: prev
}

function imitateHotReloading(TargetClass, SourceClass, container) {
// Crude imitation of hot reloading that does the job
Object.getOwnPropertyNames(SourceClass.prototype).filter(key =>
typeof SourceClass.prototype[key] === 'function'
).forEach(key => {
if (key !== 'render' && key !== 'constructor') {
TargetClass.prototype[key] = SourceClass.prototype[key]
}
})

container.forceUpdate()
}

it('should receive the store in the context', () => {
const store = createStore(() => ({}))

Expand Down Expand Up @@ -1375,28 +1388,76 @@ describe('React', () => {
expect(stub.props.foo).toEqual(undefined)
expect(stub.props.scooby).toEqual('doo')

function imitateHotReloading(TargetClass, SourceClass) {
// Crude imitation of hot reloading that does the job
Object.getOwnPropertyNames(SourceClass.prototype).filter(key =>
typeof SourceClass.prototype[key] === 'function'
).forEach(key => {
if (key !== 'render' && key !== 'constructor') {
TargetClass.prototype[key] = SourceClass.prototype[key]
}
})

container.forceUpdate()
}

imitateHotReloading(ContainerBefore, ContainerAfter)
imitateHotReloading(ContainerBefore, ContainerAfter, container)
expect(stub.props.foo).toEqual('baz')
expect(stub.props.scooby).toEqual('foo')

imitateHotReloading(ContainerBefore, ContainerNext)
imitateHotReloading(ContainerBefore, ContainerNext, container)
expect(stub.props.foo).toEqual('bar')
expect(stub.props.scooby).toEqual('boo')
})

it('should persist listeners through hot update', () => {
const ACTION_TYPE = "ACTION"
const store = createStore((state = {actions: 0}, action) => {
switch (action.type) {
case ACTION_TYPE: {
return {
actions: state.actions + 1
}
}
default:
return state
}
})

@connect(
(state) => ({ actions: state.actions })
)
class Child extends Component {
render() {
return <Passthrough {...this.props}/>
}
}

@connect(
() => ({ scooby: 'doo' })
)
class ParentBefore extends Component {
render() {
return (
<Child />
)
}
}

@connect(
() => ({ scooby: 'boo' })
)
class ParentAfter extends Component {
render() {
return (
<Child />
)
}
}

let container
TestUtils.renderIntoDocument(
<ProviderMock store={store}>
<ParentBefore ref={instance => container = instance}/>
</ProviderMock>
)

const stub = TestUtils.findRenderedComponentWithType(container, Passthrough)

imitateHotReloading(ParentBefore, ParentAfter, container)

store.dispatch({type: ACTION_TYPE})

expect(stub.props.actions).toEqual(1)
})

it('should set the displayName correctly', () => {
expect(connect(state => state)(
class Foo extends Component {
Expand Down

0 comments on commit 46d8b25

Please sign in to comment.