From 8a70b6459c8fb3dad710460a48e4b806329f8031 Mon Sep 17 00:00:00 2001 From: Pavel Leshkovich Date: Wed, 6 Nov 2019 17:21:55 +0300 Subject: [PATCH 1/4] fix performance createListenerCollection --- src/utils/Subscription.js | 43 +++++++++++++-------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js index e03f4838a..ac85b643c 100644 --- a/src/utils/Subscription.js +++ b/src/utils/Subscription.js @@ -4,46 +4,31 @@ 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. + var 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 = [] + var current = {} + var id = 0 return { - clear() { - next = CLEARED - current = CLEARED + clear: function clear() { + current = {} }, - - notify() { - const listeners = (current = next) - batch(() => { - for (let i = 0; i < listeners.length; i++) { - listeners[i]() + notify: function notify() { + var listeners = current + batch(function() { + for (const id in listeners) { + listeners[id]() } }) }, - - get() { - return next - }, - - subscribe(listener) { - let isSubscribed = true - if (next === current) next = current.slice() - next.push(listener) - + subscribe: function subscribe(listener) { + var currentId = id++ + current[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 current[currentId] } } } From 599962e0f955f819714b0efa4fc52c8aff122abd Mon Sep 17 00:00:00 2001 From: Pavel Leshkovich Date: Wed, 6 Nov 2019 17:40:45 +0300 Subject: [PATCH 2/4] fix get for test --- src/utils/Subscription.js | 5 +++++ test/hooks/useSelector.spec.js | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js index ac85b643c..75b841d0a 100644 --- a/src/utils/Subscription.js +++ b/src/utils/Subscription.js @@ -24,6 +24,11 @@ function createListenerCollection() { } }) }, + + get() { + return current + }, + subscribe: function subscribe(listener) { var currentId = id++ current[currentId] = listener 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', () => { From 2f57bcc02e3a385f5f060ea388789595884ce25e Mon Sep 17 00:00:00 2001 From: Pavel Leshkovich Date: Wed, 6 Nov 2019 18:01:29 +0300 Subject: [PATCH 3/4] small fix --- src/utils/Subscription.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js index 75b841d0a..4ee668ef9 100644 --- a/src/utils/Subscription.js +++ b/src/utils/Subscription.js @@ -7,18 +7,19 @@ import { getBatch } from './batch' const nullListeners = { notify() {} } function createListenerCollection() { - var batch = getBatch() // the current/next pattern is copied from redux's createStore code. + var batch = getBatch() + // the current/next pattern is copied from redux's createStore code. // TODO: refactor+expose that code to be reusable here? var current = {} var id = 0 return { - clear: function clear() { + clear() { current = {} }, - notify: function notify() { + notify() { var listeners = current - batch(function() { + batch(() => { for (const id in listeners) { listeners[id]() } @@ -29,7 +30,7 @@ function createListenerCollection() { return current }, - subscribe: function subscribe(listener) { + subscribe(listener) { var currentId = id++ current[currentId] = listener return function unsubscribe() { From 58a4b272a89e9a5c80b7ed1ea680e6e35edfcc93 Mon Sep 17 00:00:00 2001 From: Tim Dorr Date: Wed, 6 Nov 2019 11:09:37 -0500 Subject: [PATCH 4/4] Simplify, pretty up, and remove vars --- src/utils/Subscription.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js index 4ee668ef9..064102ea1 100644 --- a/src/utils/Subscription.js +++ b/src/utils/Subscription.js @@ -7,18 +7,16 @@ import { getBatch } from './batch' const nullListeners = { notify() {} } function createListenerCollection() { - var batch = getBatch() - // the current/next pattern is copied from redux's createStore code. - // TODO: refactor+expose that code to be reusable here? + const batch = getBatch() + let listeners = {} + let id = 0 - var current = {} - var id = 0 return { clear() { - current = {} + listeners = {} }, + notify() { - var listeners = current batch(() => { for (const id in listeners) { listeners[id]() @@ -27,14 +25,15 @@ function createListenerCollection() { }, get() { - return current + return listeners }, subscribe(listener) { - var currentId = id++ - current[currentId] = listener + const currentId = id++ + listeners[currentId] = listener + return function unsubscribe() { - delete current[currentId] + delete listeners[currentId] } } }