From e7807ef69a767033c3cfd6045a5b69314b276c2d Mon Sep 17 00:00:00 2001 From: Max Belsky Date: Sat, 4 Sep 2021 20:28:04 +0300 Subject: [PATCH] Port Subscription closure implementation from 8.x to 7.x (#1807) (#1809) --- src/components/Provider.js | 4 +- src/components/connectAdvanced.js | 4 +- src/hooks/useSelector.js | 4 +- src/utils/Subscription.js | 73 +++++++++++++++++-------------- test/hooks/useSelector.spec.js | 8 ++-- test/utils/Subscription.spec.js | 6 +-- 6 files changed, 54 insertions(+), 45 deletions(-) diff --git a/src/components/Provider.js b/src/components/Provider.js index fdfc48b75..9bae9445f 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -1,12 +1,12 @@ import React, { useMemo } from 'react' import PropTypes from 'prop-types' import { ReactReduxContext } from './Context' -import Subscription from '../utils/Subscription' +import { createSubscription } from '../utils/Subscription' import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect' function Provider({ store, context, children }) { const contextValue = useMemo(() => { - const subscription = new Subscription(store) + const subscription = createSubscription(store) subscription.onStateChange = subscription.notifyNestedSubs return { store, diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index 01f4a5553..c56e6816d 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -1,7 +1,7 @@ import hoistStatics from 'hoist-non-react-statics' import React, { useContext, useMemo, useRef, useReducer } from 'react' import { isValidElementType, isContextConsumer } from 'react-is' -import Subscription from '../utils/Subscription' +import { createSubscription } from '../utils/Subscription' import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect' import { ReactReduxContext } from './Context' @@ -334,7 +334,7 @@ export default function connectAdvanced( // This Subscription's source should match where store came from: props vs. context. A component // connected to the store via props shouldn't use subscription from context, or vice versa. - const subscription = new Subscription( + const subscription = createSubscription( store, didStoreComeFromProps ? null : contextValue.subscription ) diff --git a/src/hooks/useSelector.js b/src/hooks/useSelector.js index 85e85d13e..6df228601 100644 --- a/src/hooks/useSelector.js +++ b/src/hooks/useSelector.js @@ -1,6 +1,6 @@ import { useReducer, useRef, useMemo, useContext, useDebugValue } from 'react' import { useReduxContext as useDefaultReduxContext } from './useReduxContext' -import Subscription from '../utils/Subscription' +import { createSubscription } from '../utils/Subscription' import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect' import { ReactReduxContext } from '../components/Context' @@ -14,7 +14,7 @@ function useSelectorWithStoreAndSubscription( ) { const [, forceRender] = useReducer((s) => s + 1, 0) - const subscription = useMemo(() => new Subscription(store, contextSub), [ + const subscription = useMemo(() => createSubscription(store, contextSub), [ store, contextSub, ]) diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js index 7030ec84a..d2feef94f 100644 --- a/src/utils/Subscription.js +++ b/src/utils/Subscription.js @@ -4,8 +4,6 @@ import { getBatch } from './batch' // well as nesting subscriptions of descendant components, so that we can ensure the // ancestor components re-render before descendants -const nullListeners = { notify() {} } - function createListenerCollection() { const batch = getBatch() let first = null @@ -71,51 +69,62 @@ function createListenerCollection() { } } -export default class Subscription { - constructor(store, parentSub) { - this.store = store - this.parentSub = parentSub - this.unsubscribe = null - this.listeners = nullListeners +const nullListeners = { + notify() {}, + get: () => [], +} - this.handleChangeWrapper = this.handleChangeWrapper.bind(this) - } +export function createSubscription(store, parentSub) { + let unsubscribe + let listeners = nullListeners - addNestedSub(listener) { - this.trySubscribe() - return this.listeners.subscribe(listener) + function addNestedSub(listener) { + trySubscribe() + return listeners.subscribe(listener) } - notifyNestedSubs() { - this.listeners.notify() + function notifyNestedSubs() { + listeners.notify() } - handleChangeWrapper() { - if (this.onStateChange) { - this.onStateChange() + function handleChangeWrapper() { + if (subscription.onStateChange) { + subscription.onStateChange() } } - isSubscribed() { - return Boolean(this.unsubscribe) + function isSubscribed() { + return Boolean(unsubscribe) } - trySubscribe() { - if (!this.unsubscribe) { - this.unsubscribe = this.parentSub - ? this.parentSub.addNestedSub(this.handleChangeWrapper) - : this.store.subscribe(this.handleChangeWrapper) + function trySubscribe() { + if (!unsubscribe) { + unsubscribe = parentSub + ? parentSub.addNestedSub(handleChangeWrapper) + : store.subscribe(handleChangeWrapper) - this.listeners = createListenerCollection() + listeners = createListenerCollection() } } - tryUnsubscribe() { - if (this.unsubscribe) { - this.unsubscribe() - this.unsubscribe = null - this.listeners.clear() - this.listeners = nullListeners + function tryUnsubscribe() { + if (unsubscribe) { + unsubscribe() + unsubscribe = undefined + listeners.clear() + listeners = nullListeners } } + + const subscription = { + addNestedSub, + notifyNestedSubs, + handleChangeWrapper, + isSubscribed, + trySubscribe, + tryUnsubscribe, + getListeners: () => listeners, + } + + return subscription } diff --git a/test/hooks/useSelector.spec.js b/test/hooks/useSelector.spec.js index 17d7a80aa..2f927214e 100644 --- a/test/hooks/useSelector.spec.js +++ b/test/hooks/useSelector.spec.js @@ -101,11 +101,11 @@ describe('React', () => { ) - expect(rootSubscription.listeners.get().length).toBe(1) + expect(rootSubscription.getListeners().get().length).toBe(1) store.dispatch({ type: '' }) - expect(rootSubscription.listeners.get().length).toBe(2) + expect(rootSubscription.getListeners().get().length).toBe(2) }) it('unsubscribes when the component is unmounted', () => { @@ -129,11 +129,11 @@ describe('React', () => { ) - expect(rootSubscription.listeners.get().length).toBe(2) + expect(rootSubscription.getListeners().get().length).toBe(2) store.dispatch({ type: '' }) - expect(rootSubscription.listeners.get().length).toBe(1) + expect(rootSubscription.getListeners().get().length).toBe(1) }) it('notices store updates between render and store subscription effect', () => { diff --git a/test/utils/Subscription.spec.js b/test/utils/Subscription.spec.js index 417c81b22..0cf0d4328 100644 --- a/test/utils/Subscription.spec.js +++ b/test/utils/Subscription.spec.js @@ -1,4 +1,4 @@ -import Subscription from '../../src/utils/Subscription' +import { createSubscription } from '../../src/utils/Subscription' describe('Subscription', () => { let notifications @@ -9,13 +9,13 @@ describe('Subscription', () => { notifications = [] store = { subscribe: () => jest.fn() } - parent = new Subscription(store) + parent = createSubscription(store) parent.onStateChange = () => {} parent.trySubscribe() }) function subscribeChild(name) { - const child = new Subscription(store, parent) + const child = createSubscription(store, parent) child.onStateChange = () => notifications.push(name) child.trySubscribe() return child