Skip to content

Commit e7807ef

Browse files
authored
Port Subscription closure implementation from 8.x to 7.x (#1807) (#1809)
1 parent 2c7ef25 commit e7807ef

File tree

6 files changed

+54
-45
lines changed

6 files changed

+54
-45
lines changed

src/components/Provider.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import React, { useMemo } from 'react'
22
import PropTypes from 'prop-types'
33
import { ReactReduxContext } from './Context'
4-
import Subscription from '../utils/Subscription'
4+
import { createSubscription } from '../utils/Subscription'
55
import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect'
66

77
function Provider({ store, context, children }) {
88
const contextValue = useMemo(() => {
9-
const subscription = new Subscription(store)
9+
const subscription = createSubscription(store)
1010
subscription.onStateChange = subscription.notifyNestedSubs
1111
return {
1212
store,

src/components/connectAdvanced.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import hoistStatics from 'hoist-non-react-statics'
22
import React, { useContext, useMemo, useRef, useReducer } from 'react'
33
import { isValidElementType, isContextConsumer } from 'react-is'
4-
import Subscription from '../utils/Subscription'
4+
import { createSubscription } from '../utils/Subscription'
55
import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect'
66

77
import { ReactReduxContext } from './Context'
@@ -334,7 +334,7 @@ export default function connectAdvanced(
334334

335335
// This Subscription's source should match where store came from: props vs. context. A component
336336
// connected to the store via props shouldn't use subscription from context, or vice versa.
337-
const subscription = new Subscription(
337+
const subscription = createSubscription(
338338
store,
339339
didStoreComeFromProps ? null : contextValue.subscription
340340
)

src/hooks/useSelector.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useReducer, useRef, useMemo, useContext, useDebugValue } from 'react'
22
import { useReduxContext as useDefaultReduxContext } from './useReduxContext'
3-
import Subscription from '../utils/Subscription'
3+
import { createSubscription } from '../utils/Subscription'
44
import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect'
55
import { ReactReduxContext } from '../components/Context'
66

@@ -14,7 +14,7 @@ function useSelectorWithStoreAndSubscription(
1414
) {
1515
const [, forceRender] = useReducer((s) => s + 1, 0)
1616

17-
const subscription = useMemo(() => new Subscription(store, contextSub), [
17+
const subscription = useMemo(() => createSubscription(store, contextSub), [
1818
store,
1919
contextSub,
2020
])

src/utils/Subscription.js

Lines changed: 41 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ 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 nullListeners = { notify() {} }
8-
97
function createListenerCollection() {
108
const batch = getBatch()
119
let first = null
@@ -71,51 +69,62 @@ function createListenerCollection() {
7169
}
7270
}
7371

74-
export default class Subscription {
75-
constructor(store, parentSub) {
76-
this.store = store
77-
this.parentSub = parentSub
78-
this.unsubscribe = null
79-
this.listeners = nullListeners
72+
const nullListeners = {
73+
notify() {},
74+
get: () => [],
75+
}
8076

81-
this.handleChangeWrapper = this.handleChangeWrapper.bind(this)
82-
}
77+
export function createSubscription(store, parentSub) {
78+
let unsubscribe
79+
let listeners = nullListeners
8380

84-
addNestedSub(listener) {
85-
this.trySubscribe()
86-
return this.listeners.subscribe(listener)
81+
function addNestedSub(listener) {
82+
trySubscribe()
83+
return listeners.subscribe(listener)
8784
}
8885

89-
notifyNestedSubs() {
90-
this.listeners.notify()
86+
function notifyNestedSubs() {
87+
listeners.notify()
9188
}
9289

93-
handleChangeWrapper() {
94-
if (this.onStateChange) {
95-
this.onStateChange()
90+
function handleChangeWrapper() {
91+
if (subscription.onStateChange) {
92+
subscription.onStateChange()
9693
}
9794
}
9895

99-
isSubscribed() {
100-
return Boolean(this.unsubscribe)
96+
function isSubscribed() {
97+
return Boolean(unsubscribe)
10198
}
10299

103-
trySubscribe() {
104-
if (!this.unsubscribe) {
105-
this.unsubscribe = this.parentSub
106-
? this.parentSub.addNestedSub(this.handleChangeWrapper)
107-
: this.store.subscribe(this.handleChangeWrapper)
100+
function trySubscribe() {
101+
if (!unsubscribe) {
102+
unsubscribe = parentSub
103+
? parentSub.addNestedSub(handleChangeWrapper)
104+
: store.subscribe(handleChangeWrapper)
108105

109-
this.listeners = createListenerCollection()
106+
listeners = createListenerCollection()
110107
}
111108
}
112109

113-
tryUnsubscribe() {
114-
if (this.unsubscribe) {
115-
this.unsubscribe()
116-
this.unsubscribe = null
117-
this.listeners.clear()
118-
this.listeners = nullListeners
110+
function tryUnsubscribe() {
111+
if (unsubscribe) {
112+
unsubscribe()
113+
unsubscribe = undefined
114+
listeners.clear()
115+
listeners = nullListeners
119116
}
120117
}
118+
119+
const subscription = {
120+
addNestedSub,
121+
notifyNestedSubs,
122+
handleChangeWrapper,
123+
isSubscribed,
124+
trySubscribe,
125+
tryUnsubscribe,
126+
getListeners: () => listeners,
127+
}
128+
129+
return subscription
121130
}

test/hooks/useSelector.spec.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,11 @@ describe('React', () => {
101101
</ProviderMock>
102102
)
103103

104-
expect(rootSubscription.listeners.get().length).toBe(1)
104+
expect(rootSubscription.getListeners().get().length).toBe(1)
105105

106106
store.dispatch({ type: '' })
107107

108-
expect(rootSubscription.listeners.get().length).toBe(2)
108+
expect(rootSubscription.getListeners().get().length).toBe(2)
109109
})
110110

111111
it('unsubscribes when the component is unmounted', () => {
@@ -129,11 +129,11 @@ describe('React', () => {
129129
</ProviderMock>
130130
)
131131

132-
expect(rootSubscription.listeners.get().length).toBe(2)
132+
expect(rootSubscription.getListeners().get().length).toBe(2)
133133

134134
store.dispatch({ type: '' })
135135

136-
expect(rootSubscription.listeners.get().length).toBe(1)
136+
expect(rootSubscription.getListeners().get().length).toBe(1)
137137
})
138138

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

test/utils/Subscription.spec.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import Subscription from '../../src/utils/Subscription'
1+
import { createSubscription } from '../../src/utils/Subscription'
22

33
describe('Subscription', () => {
44
let notifications
@@ -9,13 +9,13 @@ describe('Subscription', () => {
99
notifications = []
1010
store = { subscribe: () => jest.fn() }
1111

12-
parent = new Subscription(store)
12+
parent = createSubscription(store)
1313
parent.onStateChange = () => {}
1414
parent.trySubscribe()
1515
})
1616

1717
function subscribeChild(name) {
18-
const child = new Subscription(store, parent)
18+
const child = createSubscription(store, parent)
1919
child.onStateChange = () => notifications.push(name)
2020
child.trySubscribe()
2121
return child

0 commit comments

Comments
 (0)