Skip to content

Commit

Permalink
Port Subscription closure implementation from 8.x to 7.x (#1807) (#1809)
Browse files Browse the repository at this point in the history
  • Loading branch information
mbelsky authored Sep 4, 2021
1 parent 2c7ef25 commit e7807ef
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 45 deletions.
4 changes: 2 additions & 2 deletions src/components/Provider.js
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/components/connectAdvanced.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
)
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useSelector.js
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -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,
])
Expand Down
73 changes: 41 additions & 32 deletions src/utils/Subscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
8 changes: 4 additions & 4 deletions test/hooks/useSelector.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ describe('React', () => {
</ProviderMock>
)

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', () => {
Expand All @@ -129,11 +129,11 @@ describe('React', () => {
</ProviderMock>
)

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', () => {
Expand Down
6 changes: 3 additions & 3 deletions test/utils/Subscription.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Subscription from '../../src/utils/Subscription'
import { createSubscription } from '../../src/utils/Subscription'

describe('Subscription', () => {
let notifications
Expand All @@ -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
Expand Down

0 comments on commit e7807ef

Please sign in to comment.