Skip to content

Commit

Permalink
Clear memoized for state while deferirng notifications (#261)
Browse files Browse the repository at this point in the history 
* Clear memoized for state while defering notifications

* add one more test

* rename

* rename
  • Loading branch information
@ShirShintel
ShirShintel authored Nov 6, 2024
1 parent 256a311 commit af32f3c
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 35 deletions.
6 changes: 5 additions & 1 deletion src/appHost.tsx
View file
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
View file
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
View file
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

0 comments on commit af32f3c

Please sign in to comment.