Skip to content
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

Clear memoized for state while deferring notifications #261

Merged
merged 4 commits into from
Nov 6, 2024
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
6 changes: 5 additions & 1 deletion src/appHost.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export function createAppHost(initialEntryPointsOrPackages: EntryPointOrPackage[
let isInstallingEntryPoints: boolean = false
let isStoreSubscribersNotifyInProgress = false
let isObserversNotifyInProgress = false
let shouldFlushMemoizationSync = false
const entryPointsInstallationEndCallbacks: Map<string, () => void> = new Map()

verifyLayersUniqueness(options.layers)
Expand Down Expand Up @@ -665,6 +666,9 @@ miss: ${memoizedWithMissHit.miss}
},
notifyObserversIsRunning => {
isObserversNotifyInProgress = notifyObserversIsRunning
},
updateShouldFlushMemoizationSync => {
shouldFlushMemoizationSync = updateShouldFlushMemoizationSync
}
)
store.subscribe(() => {
Expand All @@ -675,7 +679,7 @@ miss: ${memoizedWithMissHit.miss}
})
store.syncSubscribe(() => {
shouldFlushMemoization = true
if (isStoreSubscribersNotifyInProgress || isObserversNotifyInProgress) {
if (isStoreSubscribersNotifyInProgress || isObserversNotifyInProgress || shouldFlushMemoizationSync) {
shouldFlushMemoization = false
flushMemoizedForState()
}
Expand Down
15 changes: 9 additions & 6 deletions src/throttledStore.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Reducer, createStore, Store, ReducersMapObject, combineReducers, AnyAction, Dispatch, Action, Unsubscribe } from 'redux'
import _ from 'lodash'
import { Action, AnyAction, Dispatch, Reducer, ReducersMapObject, Store, Unsubscribe, combineReducers, createStore } from 'redux'
import { devToolsEnhancer } from 'redux-devtools-extension'
import { AppHost, ExtensionSlot, ObservableState, ReducersMapObjectContributor, Shell, SlotKey, StateObserver } from './API'
import { AppHostServicesProvider } from './appHostServices'
import _ from 'lodash'
import { AppHost, ExtensionSlot, ReducersMapObjectContributor, ObservableState, StateObserver, Shell, SlotKey } from './API'
import { contributeInstalledShellsState } from './installedShellsState'
import { interceptAnyObject } from './interceptAnyObject'
import { invokeSlotCallbacks } from './invokeSlotCallbacks'
Expand Down Expand Up @@ -47,7 +47,7 @@ export interface StateContribution<TState = {}, TAction extends AnyAction = AnyA
export interface ThrottledStore<T = any> extends Store<T> {
hasPendingSubscribers(): boolean
flush(config?: { excecutionType: 'scheduled' | 'immediate' | 'default' }): void
deferSubscriberNotifications<K>(action: () => K | Promise<K>): Promise<K>
deferSubscriberNotifications<K>(action: () => K | Promise<K>, shouldDispatchClearCache?: boolean): Promise<K>
}

export interface PrivateThrottledStore<T = any> extends ThrottledStore<T> {
Expand Down Expand Up @@ -157,7 +157,8 @@ export const createThrottledStore = (
requestAnimationFrame: Window['requestAnimationFrame'],
cancelAnimationFrame: Window['cancelAnimationFrame'],
updateIsSubscriptionNotifyInProgress: (isSubscriptionNotifyInProgress: boolean) => void,
updateIsObserversNotifyInProgress: (isObserversNotifyInProgress: boolean) => void
updateIsObserversNotifyInProgress: (isObserversNotifyInProgress: boolean) => void,
updateShouldFlushMemoizationSync: (shouldFlushMemoizationSync: boolean) => void
): PrivateThrottledStore => {
let pendingBroadcastNotification = false
let pendingObservableNotifications: Set<AnyPrivateObservableState> | undefined
Expand Down Expand Up @@ -279,17 +280,19 @@ export const createThrottledStore = (
observableNotify: onObservableNotify,
resetPendingNotifications: resetAllPendingNotifications,
hasPendingSubscribers: () => pendingBroadcastNotification,
deferSubscriberNotifications: async action => {
deferSubscriberNotifications: async (action, shouldDispatchClearCache) => {
if (isDeferrringNotifications) {
return action()
}
try {
executePendingActions()
isDeferrringNotifications = true
shouldDispatchClearCache && updateShouldFlushMemoizationSync(isDeferrringNotifications)
const functionResult = await action()
return functionResult
} finally {
isDeferrringNotifications = false
shouldDispatchClearCache && updateShouldFlushMemoizationSync(isDeferrringNotifications)
executePendingActions()
}
}
Expand Down
128 changes: 100 additions & 28 deletions test/connectWithShell.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import _ from 'lodash'
import React, { FunctionComponent, ReactElement, useEffect } from 'react'

import { AppHost, EntryPoint, Shell, SlotKey, ObservableState, AnySlotKey } from '../src/API'
import { act, create, ReactTestInstance, ReactTestRenderer } from 'react-test-renderer'
import { AnyAction } from 'redux'
import { ObservedSelectorsMap, observeWithShell } from '../src'
import { AnySlotKey, AppHost, EntryPoint, ObservableState, Shell, SlotKey } from '../src/API'
import {
collectAllTexts,
connectWithShell,
connectWithShellAndObserve,
createAppHost,
mockPackage,
mockShellStateKey,
MockState,
renderInHost,
connectWithShell,
connectWithShellAndObserve,
withThrowOnError,
TOGGLE_MOCK_VALUE,
collectAllTexts
withThrowOnError
} from '../testKit'
import { ReactTestRenderer, act, create, ReactTestInstance } from 'react-test-renderer'
import { AnyAction } from 'redux'
import { ObservedSelectorsMap, observeWithShell } from '../src'

interface MockPackageState {
[mockShellStateKey]: MockState
Expand Down Expand Up @@ -59,7 +59,9 @@ describe('connectWithShell', () => {
const PureComp = ({ shellName }: { shellName: string }) => <div>{shellName}</div>
const mapStateToProps = (s: Shell) => ({ shellName: s.name })

const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { parentWrapper } = renderInShellContext(<ConnectedComp />)
expect(collectAllTexts(parentWrapper)).toContain(mockPackage.name)
Expand Down Expand Up @@ -128,7 +130,9 @@ describe('connectWithShell', () => {
return <div onClick={func}>{JSON.stringify(obj)}</div>
}

const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)

Expand Down Expand Up @@ -382,7 +386,9 @@ describe('connectWithShell', () => {
// Assert - outer component re-rendered and passed new ownProps to inner component
expect(mapStateOuterCompSpy).toHaveBeenCalledTimes(2)
expect(outerComponentRenderSpy).toHaveBeenCalledTimes(2)
expect(innerCompShouldComponentUpdateSpy).toHaveBeenCalledWith({ num: 2 })
expect(innerCompShouldComponentUpdateSpy).toHaveBeenCalledWith({
num: 2
})

// Assert - should not trigger mapDispatchToProps or re-render of inner component even though it's ownProps have changed
expect(mapDispatchInnerCompSpy).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -497,7 +503,9 @@ describe('connectWithShell', () => {
MockPackageState,
PureCompWithChildrenOwnProps,
PureCompWithChildrenStateProps
>(mapStateToProps, undefined, getBoundShell(), { allowOutOfEntryPoint: true })(PureCompWithChildren)
>(mapStateToProps, undefined, getBoundShell(), {
allowOutOfEntryPoint: true
})(PureCompWithChildren)

const { testKit } = renderInShellContext(
<ConnectedUnboundCompWithChildren id="A">
Expand All @@ -521,7 +529,9 @@ describe('connectWithShell', () => {
}
})
const PureComp: FunctionComponent<{}> = () => <div className="TEST-PURE-COMP">TEST</div>
const ConnectedComp = connectWithShell(undefined, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(undefined, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

// act
const { testKit } = renderInHost(<ConnectedComp />, host, shell)
Expand All @@ -542,7 +552,9 @@ describe('connectWithShell', () => {
}
})
const PureComp: FunctionComponent<{}> = () => <div className="TEST-PURE-COMP">TEST</div>
const ConnectedComp = connectWithShell(undefined, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(undefined, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

// act

Expand Down Expand Up @@ -572,7 +584,9 @@ describe('connectWithShell', () => {
const PureComp: FunctionComponent<{}> = () => (
<TestAspectContext.Consumer>{aspect => <div className="TEST-PURE-COMP">{aspect.theNumber}</div>}</TestAspectContext.Consumer>
)
const ConnectedComp = connectWithShell(undefined, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(undefined, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

// act

Expand Down Expand Up @@ -601,23 +615,32 @@ describe('connectWithShell-useCases', () => {
interface SecondStateAPI {
getValueTwo(): string
}
const SecondStateAPI: SlotKey<SecondStateAPI> = { name: 'TWO_API', public: true }
const SecondStateAPI: SlotKey<SecondStateAPI> = {
name: 'TWO_API',
public: true
}

interface FirstObservableAPI {
observables: { three: ObservableState<FirstObservableSelectors> }
}
interface FirstObservableSelectors {
getValueThree(): string
}
const FirstObservableAPI: SlotKey<FirstObservableAPI> = { name: 'THREE_API', public: true }
const FirstObservableAPI: SlotKey<FirstObservableAPI> = {
name: 'THREE_API',
public: true
}

interface SecondObservableAPI {
observables: { four: ObservableState<SecondObservableSelectors> }
}
interface SecondObservableSelectors {
getValueFour(): string
}
const SecondObservableAPI: SlotKey<SecondObservableAPI> = { name: 'FOUR_API', public: true }
const SecondObservableAPI: SlotKey<SecondObservableAPI> = {
name: 'FOUR_API',
public: true
}

const entryPointWithState: EntryPoint = {
name: 'ONE',
Expand Down Expand Up @@ -791,7 +814,9 @@ describe('connectWithShell-useCases', () => {

it('should not notify subscribers when deferring notifications', async () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand Down Expand Up @@ -819,7 +844,9 @@ describe('connectWithShell-useCases', () => {

it('should not have pending subscribers when starting to defer notifications', async () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand All @@ -839,7 +866,9 @@ describe('connectWithShell-useCases', () => {

it('should notify subscribers of state changes before deferring notifications', async () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand All @@ -857,7 +886,9 @@ describe('connectWithShell-useCases', () => {

it('should notify after action failed while deferring notifications', async () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand All @@ -884,7 +915,9 @@ describe('connectWithShell-useCases', () => {

it('should flush while deferring notifications if immediate flush was called during that action', async () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand All @@ -906,7 +939,9 @@ describe('connectWithShell-useCases', () => {

it('should support nested defered notification actions', async () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand Down Expand Up @@ -937,9 +972,39 @@ describe('connectWithShell-useCases', () => {
expect(renderSpy).toHaveBeenCalledTimes(2)
})

it('should clear state memoization on dispatch when deferring notifications and setting shouldDispatchClearCache', async () => {
const { host, shell } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
let numberOfCalls = 0
const originalFn = jest.fn(() => ++numberOfCalls)
const memoizedFn = shell.memoizeForState(originalFn, () => '*') as _.MemoizedFunction
const clearCacheSpy = jest.spyOn(memoizedFn.cache, 'clear')

await host.getStore().deferSubscriberNotifications(() => {
host.getStore().dispatch({ type: 'MOCK' })
}, true)

expect(clearCacheSpy).toHaveBeenCalledTimes(1)
})

it('should not clear state memoization on dispatch when deferring notifications without setting shouldDispatchClearCache', async () => {
const { host, shell } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
let numberOfCalls = 0
const originalFn = jest.fn(() => ++numberOfCalls)
const memoizedFn = shell.memoizeForState(originalFn, () => '*') as _.MemoizedFunction
const clearCacheSpy = jest.spyOn(memoizedFn.cache, 'clear')

await host.getStore().deferSubscriberNotifications(() => {
host.getStore().dispatch({ type: 'MOCK' })
})

expect(clearCacheSpy).toHaveBeenCalledTimes(0)
})

it('should not mount connected component on props update', () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)
const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
throw new Error('Connected component failed to render')
Expand All @@ -956,7 +1021,9 @@ describe('connectWithShell-useCases', () => {

it('should update component on change in regular state', () => {
const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand Down Expand Up @@ -995,7 +1062,9 @@ describe('connectWithShell-useCases', () => {
entryPointSecondStateWithAPI,
entryPointFirstObservable
])
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp)
const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, {
allowOutOfEntryPoint: true
})(PureComp)

const { testKit } = renderInShellContext(<ConnectedComp />)
if (!testKit) {
Expand Down Expand Up @@ -1210,7 +1279,10 @@ describe('observeWithShellPureComponent', () => {
getStringValue(): string
getNumberValue(): number
}
const ObservableAPI: SlotKey<ObservableAPI> = { name: 'OBSERVABLE_API', public: true }
const ObservableAPI: SlotKey<ObservableAPI> = {
name: 'OBSERVABLE_API',
public: true
}
interface ActualObservableState {
stringValue: string
numberValue: number
Expand Down
Loading