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

Persist Nested Subscriptions Through Hot Reload #715

Merged
merged 1 commit into from
Jul 27, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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