Skip to content

Commit 476c0de

Browse files
leshkovichpvltimdorr
authored andcommitted
fix performance createListenerCollection (reduxjs#1450)
* fix performance createListenerCollection * fix get for test * small fix * Simplify, pretty up, and remove vars
1 parent 82b39ef commit 476c0de

File tree

2 files changed

+13
-23
lines changed

2 files changed

+13
-23
lines changed

src/utils/Subscription.js

+9-19
Original file line numberDiff line numberDiff line change
@@ -4,46 +4,36 @@ import { getBatch } from './batch'
44
// well as nesting subscriptions of descendant components, so that we can ensure the
55
// ancestor components re-render before descendants
66

7-
const CLEARED = null
87
const nullListeners = { notify() {} }
98

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

1714
return {
1815
clear() {
19-
next = CLEARED
20-
current = CLEARED
16+
listeners = {}
2117
},
2218

2319
notify() {
24-
const listeners = (current = next)
2520
batch(() => {
26-
for (let i = 0; i < listeners.length; i++) {
27-
listeners[i]()
21+
for (const id in listeners) {
22+
listeners[id]()
2823
}
2924
})
3025
},
3126

3227
get() {
33-
return next
28+
return listeners
3429
},
3530

3631
subscribe(listener) {
37-
let isSubscribed = true
38-
if (next === current) next = current.slice()
39-
next.push(listener)
32+
const currentId = id++
33+
listeners[currentId] = listener
4034

4135
return function unsubscribe() {
42-
if (!isSubscribed || current === CLEARED) return
43-
isSubscribed = false
44-
45-
if (next === current) next = current.slice()
46-
next.splice(next.indexOf(listener), 1)
36+
delete listeners[currentId]
4737
}
4838
}
4939
}

test/hooks/useSelector.spec.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@ describe('React', () => {
9797
</ProviderMock>
9898
)
9999

100-
expect(rootSubscription.listeners.get().length).toBe(1)
100+
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(1)
101101

102102
store.dispatch({ type: '' })
103103

104-
expect(rootSubscription.listeners.get().length).toBe(2)
104+
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(2)
105105
})
106106

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

128-
expect(rootSubscription.listeners.get().length).toBe(2)
128+
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(2)
129129

130130
store.dispatch({ type: '' })
131131

132-
expect(rootSubscription.listeners.get().length).toBe(1)
132+
expect(Object.keys(rootSubscription.listeners.get()).length).toBe(1)
133133
})
134134

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

0 commit comments

Comments
 (0)