Skip to content

Rewrite Subscription as a closure factory for byte shaving #1755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .babelrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@ module.exports = {
// Use the equivalent of `babel-preset-modules`
bugfixes: true,
modules: false,
loose: true,
},
],
'@babel/preset-typescript',
],
plugins: [
['@babel/proposal-decorators', { legacy: true }],
'@babel/transform-react-jsx',
['@babel/plugin-proposal-class-properties', { loose: true }],
['@babel/plugin-proposal-private-methods', { loose: true }],
['@babel/plugin-proposal-private-property-in-object', { loose: true }],
cjs && ['@babel/transform-modules-commonjs'],
[
'@babel/transform-runtime',
Expand Down
7 changes: 3 additions & 4 deletions src/components/Context.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react'
import { Action, AnyAction, Store } from 'redux'
import type { FixTypeLater } from '../types'
import type Subscription from '../utils/Subscription'
import type { Subscription } from '../utils/Subscription'

export interface ReactReduxContextValue<
SS = FixTypeLater,
Expand All @@ -11,9 +11,8 @@ export interface ReactReduxContextValue<
subscription: Subscription
}

export const ReactReduxContext = /*#__PURE__*/ React.createContext<ReactReduxContextValue | null>(
null
)
export const ReactReduxContext =
/*#__PURE__*/ React.createContext<ReactReduxContextValue | null>(null)

if (process.env.NODE_ENV !== 'production') {
ReactReduxContext.displayName = 'ReactRedux'
Expand Down
4 changes: 2 additions & 2 deletions src/components/Provider.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { Context, ReactNode, useMemo } from 'react'
import { ReactReduxContext, ReactReduxContextValue } from './Context'
import Subscription from '../utils/Subscription'
import { createSubscription } from '../utils/Subscription'
import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect'
import type { FixTypeLater } from '../types'
import { Action, AnyAction, Store } from 'redux'
Expand All @@ -21,7 +21,7 @@ interface ProviderProps<A extends Action = AnyAction> {

function Provider({ store, context, children }: ProviderProps) {
const contextValue = useMemo(() => {
const subscription = new Subscription(store)
const subscription = createSubscription(store)
subscription.onStateChange = subscription.notifyNestedSubs
return {
store,
Expand Down
9 changes: 4 additions & 5 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 All @@ -343,9 +343,8 @@ export default function connectAdvanced(
// the middle of the notification loop, where `subscription` will then be null. This can
// probably be avoided if Subscription's listeners logic is changed to not call listeners
// that have been unsubscribed in the middle of the notification loop.
const notifyNestedSubs = subscription.notifyNestedSubs.bind(
subscription
)
const notifyNestedSubs =
subscription.notifyNestedSubs.bind(subscription)

return [subscription, notifyNestedSubs]
}, [store, didStoreComeFromProps, contextValue])
Expand Down
28 changes: 18 additions & 10 deletions src/hooks/useSelector.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { useReducer, useRef, useMemo, useContext, useDebugValue } from 'react'
import { useReduxContext as useDefaultReduxContext } from './useReduxContext'
import Subscription from '../utils/Subscription'
import { createSubscription, Subscription } from '../utils/Subscription'
import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect'
import { ReactReduxContext } from '../components/Context'
import { AnyAction, Store } from 'redux'
import { DefaultRootState } from '../types'

type EqualityFn<T> = (a: T | undefined, b: T | undefined) => boolean;
type EqualityFn<T> = (a: T | undefined, b: T | undefined) => boolean

const refEquality: EqualityFn<any> = (a, b) => a === b

type TSelector<S, R> = (state: S) => R;
type TSelector<S, R> = (state: S) => R

function useSelectorWithStoreAndSubscription<TStoreState, TSelectedState>(
selector: TSelector<TStoreState, TSelectedState>,
Expand All @@ -20,10 +20,10 @@ function useSelectorWithStoreAndSubscription<TStoreState, TSelectedState>(
): TSelectedState {
const [, forceRender] = useReducer((s) => s + 1, 0)

const subscription = useMemo(() => new Subscription(store, contextSub), [
store,
contextSub,
])
const subscription = useMemo(
() => createSubscription(store, contextSub),
[store, contextSub]
)

const latestSubscriptionCallbackError = useRef<Error>()
const latestSelector = useRef<TSelector<TStoreState, TSelectedState>>()
Expand Down Expand Up @@ -107,13 +107,21 @@ function useSelectorWithStoreAndSubscription<TStoreState, TSelectedState>(
* @param {React.Context} [context=ReactReduxContext] Context passed to your `<Provider>`.
* @returns {Function} A `useSelector` hook bound to the specified context.
*/
export function createSelectorHook(context = ReactReduxContext): <TState = DefaultRootState, Selected = unknown>(selector: (state: TState) => Selected, equalityFn?: EqualityFn<Selected>) => Selected {
export function createSelectorHook(
context = ReactReduxContext
): <TState = DefaultRootState, Selected = unknown>(
selector: (state: TState) => Selected,
equalityFn?: EqualityFn<Selected>
) => Selected {
const useReduxContext =
context === ReactReduxContext
? useDefaultReduxContext
: () => useContext(context)

return function useSelector<TState, Selected extends unknown>(selector: (state: TState) => Selected, equalityFn: EqualityFn<Selected> = refEquality): Selected {

return function useSelector<TState, Selected extends unknown>(
selector: (state: TState) => Selected,
equalityFn: EqualityFn<Selected> = refEquality
): Selected {
if (process.env.NODE_ENV !== 'production') {
if (!selector) {
throw new Error(`You must pass a selector to useSelector`)
Expand Down
90 changes: 55 additions & 35 deletions src/utils/Subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import { getBatch } from './batch'
// well as nesting subscriptions of descendant components, so that we can ensure the
// ancestor components re-render before descendants

type VoidFunc = () => void

type Listener = {
callback: () => void
callback: VoidFunc
next: Listener | null
prev: Listener | null
}
Expand Down Expand Up @@ -77,55 +79,73 @@ function createListenerCollection() {

type ListenerCollection = ReturnType<typeof createListenerCollection>

export default class Subscription {
private store: any
private parentSub?: Subscription
private unsubscribe?: () => void
private listeners?: ListenerCollection
public onStateChange?: () => void
export interface Subscription {
addNestedSub: (listener: VoidFunc) => VoidFunc
notifyNestedSubs: VoidFunc
handleChangeWrapper: VoidFunc
isSubscribed: () => boolean
onStateChange?: VoidFunc
trySubscribe: VoidFunc
tryUnsubscribe: VoidFunc
getListeners: () => ListenerCollection
}

constructor(store: any, parentSub?: Subscription) {
this.store = store
this.parentSub = parentSub
this.unsubscribe = undefined
this.listeners = undefined
const nullListeners = {
notify() {},
get: () => [],
} as unknown as ListenerCollection

this.handleChangeWrapper = this.handleChangeWrapper.bind(this)
}
export function createSubscription(store: any, parentSub?: Subscription) {
let unsubscribe: VoidFunc | undefined
let listeners: ListenerCollection = nullListeners

addNestedSub(listener: () => void) {
this.trySubscribe()
return this.listeners?.subscribe(listener)
function addNestedSub(listener: () => void) {
trySubscribe()
return listeners.subscribe(listener)
}

notifyNestedSubs() {
this.listeners?.notify()
function notifyNestedSubs() {
listeners.notify()
}

handleChangeWrapper() {
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 = undefined
this.listeners?.clear()
this.listeners = undefined
function tryUnsubscribe() {
if (unsubscribe) {
unsubscribe()
unsubscribe = undefined
listeners.clear()
listeners = nullListeners
}
}

const subscription: Subscription = {
addNestedSub,
notifyNestedSubs,
handleChangeWrapper,
isSubscribed,
trySubscribe,
tryUnsubscribe,
getListeners: () => listeners,
}

return subscription
}
24 changes: 3 additions & 21 deletions src/utils/bindActionCreators.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,10 @@
import {
ActionCreator,
ActionCreatorsMapObject,
AnyAction,
Dispatch,
} from 'redux'

function bindActionCreator<A extends AnyAction = AnyAction>(
actionCreator: ActionCreator<A>,
dispatch: Dispatch
) {
return function (this: any, ...args: any[]) {
return dispatch(actionCreator.apply(this, args))
}
}
import { ActionCreatorsMapObject, Dispatch } from 'redux'

export default function bindActionCreators(
actionCreators: ActionCreator<any> | ActionCreatorsMapObject,
actionCreators: ActionCreatorsMapObject,
dispatch: Dispatch
) {
if (typeof actionCreators === 'function') {
return bindActionCreator(actionCreators, dispatch)
}

const boundActionCreators: ActionCreatorsMapObject = {}
const boundActionCreators: ActionCreatorsMapObject<any> = {}
for (const key in actionCreators) {
const actionCreator = actionCreators[key]
if (typeof actionCreator === 'function') {
Expand Down
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