From 476c0de5d20acd45fbcea09fe7e52304184cc274 Mon Sep 17 00:00:00 2001 From: leshkovichpvl Date: Wed, 6 Nov 2019 19:13:45 +0300 Subject: [PATCH] fix performance createListenerCollection (#1450) * fix performance createListenerCollection * fix get for test * small fix * Simplify, pretty up, and remove vars --- src/utils/Subscription.js | 28 +++++++++------------------- test/hooks/useSelector.spec.js | 8 ++++---- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js index e03f4838a..064102ea1 100644 --- a/src/utils/Subscription.js +++ b/src/utils/Subscription.js @@ -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] } } } diff --git a/test/hooks/useSelector.spec.js b/test/hooks/useSelector.spec.js index b0c8692b0..51fb76532 100644 --- a/test/hooks/useSelector.spec.js +++ b/test/hooks/useSelector.spec.js @@ -97,11 +97,11 @@ describe('React', () => { ) - 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', () => { @@ -125,11 +125,11 @@ describe('React', () => { ) - 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', () => {