Skip to content

Commit

Permalink
fix performance createListenerCollection (#1450)
Browse files Browse the repository at this point in the history
* fix performance createListenerCollection

* fix get for test

* small fix

* Simplify, pretty up, and remove vars
  • Loading branch information
leshkovichpvl authored and timdorr committed Nov 6, 2019
1 parent 82b39ef commit 476c0de
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 23 deletions.
28 changes: 9 additions & 19 deletions src/utils/Subscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,46 +4,36 @@ import { getBatch } from './batch'
// well as nesting subscriptions of descendant components, so that we can ensure the
// ancestor components re-render before descendants

const CLEARED = null
const nullListeners = { notify() {} }

function createListenerCollection() {
const batch = getBatch()
// the current/next pattern is copied from redux's createStore code.
// TODO: refactor+expose that code to be reusable here?
let current = []
let next = []
let listeners = {}
let id = 0

return {
clear() {
next = CLEARED
current = CLEARED
listeners = {}
},

notify() {
const listeners = (current = next)
batch(() => {
for (let i = 0; i < listeners.length; i++) {
listeners[i]()
for (const id in listeners) {
listeners[id]()
}
})
},

get() {
return next
return listeners
},

subscribe(listener) {
let isSubscribed = true
if (next === current) next = current.slice()
next.push(listener)
const currentId = id++
listeners[currentId] = listener

return function unsubscribe() {
if (!isSubscribed || current === CLEARED) return
isSubscribed = false

if (next === current) next = current.slice()
next.splice(next.indexOf(listener), 1)
delete listeners[currentId]
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions test/hooks/useSelector.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ describe('React', () => {
</ProviderMock>
)

expect(rootSubscription.listeners.get().length).toBe(1)
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(1)

store.dispatch({ type: '' })

expect(rootSubscription.listeners.get().length).toBe(2)
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(2)
})

it('unsubscribes when the component is unmounted', () => {
Expand All @@ -125,11 +125,11 @@ describe('React', () => {
</ProviderMock>
)

expect(rootSubscription.listeners.get().length).toBe(2)
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(2)

store.dispatch({ type: '' })

expect(rootSubscription.listeners.get().length).toBe(1)
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(1)
})

it('notices store updates between render and store subscription effect', () => {
Expand Down

0 comments on commit 476c0de

Please sign in to comment.