From e649fb6d79b8d06d4f9d47bcff9a3b259f9897eb Mon Sep 17 00:00:00 2001 From: wurstbonbon <3285994+wurstbonbon@users.noreply.github.com> Date: Tue, 18 Feb 2020 04:03:13 +0100 Subject: [PATCH] Optimize createListenerCollection (#1523) --- src/utils/Subscription.js | 56 ++++++++++++++++++++++---------- test/utils/Subscription.spec.js | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 17 deletions(-) create mode 100644 test/utils/Subscription.spec.js diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js index e03f4838a..9e26f7ac0 100644 --- a/src/utils/Subscription.js +++ b/src/utils/Subscription.js @@ -4,46 +4,68 @@ 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 first = null + let last = null return { clear() { - next = CLEARED - current = CLEARED + first = null + last = null }, notify() { - const listeners = (current = next) batch(() => { - for (let i = 0; i < listeners.length; i++) { - listeners[i]() + let listener = first + while (listener) { + listener.callback() + listener = listener.next } }) }, get() { - return next + let listeners = [] + let listener = first + while (listener) { + listeners.push(listener) + listener = listener.next + } + return listeners }, - subscribe(listener) { + subscribe(callback) { let isSubscribed = true - if (next === current) next = current.slice() - next.push(listener) + + let listener = (last = { + callback, + next: null, + prev: last + }) + + if (listener.prev) { + listener.prev.next = listener + } else { + first = listener + } return function unsubscribe() { - if (!isSubscribed || current === CLEARED) return + if (!isSubscribed || first === null) return isSubscribed = false - if (next === current) next = current.slice() - next.splice(next.indexOf(listener), 1) + if (listener.next) { + listener.next.prev = listener.prev + } else { + last = listener.prev + } + if (listener.prev) { + listener.prev.next = listener.next + } else { + first = listener.next + } } } } diff --git a/test/utils/Subscription.spec.js b/test/utils/Subscription.spec.js new file mode 100644 index 000000000..417c81b22 --- /dev/null +++ b/test/utils/Subscription.spec.js @@ -0,0 +1,57 @@ +import Subscription from '../../src/utils/Subscription' + +describe('Subscription', () => { + let notifications + let store + let parent + + beforeEach(() => { + notifications = [] + store = { subscribe: () => jest.fn() } + + parent = new Subscription(store) + parent.onStateChange = () => {} + parent.trySubscribe() + }) + + function subscribeChild(name) { + const child = new Subscription(store, parent) + child.onStateChange = () => notifications.push(name) + child.trySubscribe() + return child + } + + it('listeners are notified in order', () => { + subscribeChild('child1') + subscribeChild('child2') + subscribeChild('child3') + subscribeChild('child4') + + parent.notifyNestedSubs() + + expect(notifications).toEqual(['child1', 'child2', 'child3', 'child4']) + }) + + it('listeners can be unsubscribed', () => { + const child1 = subscribeChild('child1') + const child2 = subscribeChild('child2') + const child3 = subscribeChild('child3') + + child2.tryUnsubscribe() + parent.notifyNestedSubs() + + expect(notifications).toEqual(['child1', 'child3']) + notifications.length = 0 + + child1.tryUnsubscribe() + parent.notifyNestedSubs() + + expect(notifications).toEqual(['child3']) + notifications.length = 0 + + child3.tryUnsubscribe() + parent.notifyNestedSubs() + + expect(notifications).toEqual([]) + }) +})