Skip to content

[WIP] Proof of concept of batching updates #293

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed
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
62 changes: 61 additions & 1 deletion src/components/Provider.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Component, PropTypes, Children } from 'react'
import storeShape from '../utils/storeShape'
import batchedUpdates from '../utils/batchedUpdates'

let didWarnAboutReceivingStore = false
function warnAboutReceivingStore() {
Expand All @@ -21,14 +22,73 @@ function warnAboutReceivingStore() {
/* eslint-disable no-console */
}

function batchListenerCalls(store) {
let currentListeners = []
let nextListeners = currentListeners

const ensureCanMutateNextListeners = () => {
if (nextListeners === currentListeners) {
nextListeners = currentListeners.slice()
}
}

let notifyListeners = () => {
const listeners = currentListeners = nextListeners
for (let i = 0; i < listeners.length; i++) {
listeners[i]()
}
}

let batchListener = () => {
batchedUpdates(notifyListeners)
}

let unsubscribeBatchListener

return {
...store,
subscribe(listener) {
if (typeof listener !== 'function') {
throw new Error('Expected listener to be a function.')
}

let isSubscribed = true

ensureCanMutateNextListeners()
nextListeners.push(listener)

if (!unsubscribeBatchListener) {
unsubscribeBatchListener = store.subscribe(batchListener)
}

return () => {
if (!isSubscribed) {
return
}

isSubscribed = false

ensureCanMutateNextListeners()
const index = nextListeners.indexOf(listener)
nextListeners.splice(index, 1)

if (!nextListeners.length && unsubscribeBatchListener) {
unsubscribeBatchListener()
unsubscribeBatchListener = null
}
}
}
}
}

export default class Provider extends Component {
getChildContext() {
return { store: this.store }
}

constructor(props, context) {
super(props, context)
this.store = props.store
this.store = batchListenerCalls(props.store)
}

render() {
Expand Down
1 change: 1 addition & 0 deletions src/utils/batchedUpdates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { unstable_batchedUpdates as default } from 'react-dom'
1 change: 1 addition & 0 deletions src/utils/batchedUpdates.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { unstable_batchedUpdates as default } from 'react-native'
73 changes: 71 additions & 2 deletions test/components/Provider.spec.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import expect from 'expect'
import React, { PropTypes, Component } from 'react'
import ReactDOM from 'react-dom'
import TestUtils from 'react-addons-test-utils'
import { createStore } from 'redux'
import { Provider } from '../../src/index'
import { Provider, connect } from '../../src/index'

describe('React', () => {
describe('Provider', () => {
Expand Down Expand Up @@ -60,7 +61,10 @@ describe('React', () => {
expect(spy.calls.length).toBe(0)

const child = TestUtils.findRenderedComponentWithType(tree, Child)
expect(child.context.store).toBe(store)
expect(child.context.store).toExist()
expect(child.context.store.dispatch).toBeA('function')
expect(child.context.store.getState).toBeA('function')
expect(child.context.store.subscribe).toBeA('function')
})

it('should warn once when receiving a new store in props', () => {
Expand Down Expand Up @@ -107,5 +111,70 @@ describe('React', () => {
expect(child.context.store.getState()).toEqual(11)
expect(spy.calls.length).toBe(0)
})

it('should pass state consistently to mapState', () => {
function stringBuilder(prev = '', action) {
return action.type === 'APPEND'
? prev + action.body
: prev
}

const store = createStore(stringBuilder)

store.dispatch({ type: 'APPEND', body: 'a' })
let childMapStateInvokes = 0

@connect(state => ({ state }), null, null, { withRef: true })
class Container extends Component {
emitChange() {
store.dispatch({ type: 'APPEND', body: 'b' })
}

render() {
return (
<div>
<button ref="button" onClick={this.emitChange.bind(this)}>change</button>
<ChildContainer parentState={this.props.state} />
</div>
)
}
}

@connect((state, parentProps) => {
childMapStateInvokes++
// The state from parent props should always be consistent with the current state
expect(state).toEqual(parentProps.parentState)
return {}
})
class ChildContainer extends Component {
render() {
return <div {...this.props} />
}
}

const tree = TestUtils.renderIntoDocument(
<Provider store={store}>
<Container />
</Provider>
)

expect(childMapStateInvokes).toBe(1)

// The store state stays consistent when setState calls are batched
ReactDOM.unstable_batchedUpdates(() => {
store.dispatch({ type: 'APPEND', body: 'c' })
})
expect(childMapStateInvokes).toBe(2)

// setState calls DOM handlers are batched
const container = TestUtils.findRenderedComponentWithType(tree, Container)
const node = container.getWrappedInstance().refs.button
TestUtils.Simulate.click(node)
expect(childMapStateInvokes).toBe(3)

// Provider uses unstable_batchedUpdates() under the hood
store.dispatch({ type: 'APPEND', body: 'd' })
expect(childMapStateInvokes).toBe(4)
})
})
})
8 changes: 4 additions & 4 deletions test/components/connect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1468,13 +1468,13 @@ describe('React', () => {
expect(childMapStateInvokes).toBe(3)

// In future all setState calls will be batched[1]. Uncomment when it
// happens. For now redux-batched-updates middleware can be used as
// workaround this.
// happens. For now you can use <Provider> that takes care of this.
//
// [1]: https://twitter.com/sebmarkbage/status/642366976824864768
//
// store.dispatch({ type: 'APPEND', body: 'd' })
// expect(childMapStateInvokes).toBe(4)
expect(() => {
store.dispatch({ type: 'APPEND', body: 'd' })
}).toThrow(`Expected 'acbd' to equal 'acb'`)
})

it('should not render the wrapped component when mapState does not produce change', () => {
Expand Down