From 2b31034989060a1e67b50b1e007def405db85ca3 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Wed, 27 Dec 2023 18:14:43 +0200 Subject: [PATCH 01/13] expose deferSubscriberNotifications on the shell --- src/API.ts | 8 ++++++++ src/appHost.tsx | 15 ++++++++++++++- src/throttledStore.tsx | 8 +++++++- testKit/index.tsx | 3 ++- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/API.ts b/src/API.ts index dd2ddb72..cb40b8ac 100644 --- a/src/API.ts +++ b/src/API.ts @@ -471,6 +471,14 @@ export interface Shell extends Pick & Partial)} memoizedFunction */ clearCache(memoizedFunction: Partial<_.MemoizedFunction> & Partial): void + /** + * Defer subscribers notification while performing a function + * + * @param func + */ + deferSubscriberNotifications( + func: () => T | Promise, + ): Promise } export interface PrivateShell extends Shell { diff --git a/src/appHost.tsx b/src/appHost.tsx index 4201f003..b52aea05 100644 --- a/src/appHost.tsx +++ b/src/appHost.tsx @@ -1090,7 +1090,20 @@ miss: ${memoizedWithMissHit.miss} wrapWithShellRenderer(component): JSX.Element { return - } + }, + + deferSubscriberNotifications: async (func) => { + try { + (host.getStore() as PrivateThrottledStore).deferNotifications(true); + const functionResult = await func(); + return functionResult; + + } + finally { + (host.getStore() as PrivateThrottledStore).deferNotifications(false); + host.getStore().flush(); + } + }, } return shell diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index 65d6b05f..d3746bd8 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -55,6 +55,7 @@ export interface PrivateThrottledStore extends ThrottledStore { resetPendingNotifications(): void syncSubscribe(listener: () => void): Unsubscribe dispatchWithShell(shell: Shell): Dispatch + deferNotifications(flag: boolean): void } export interface PrivateObservableState extends ObservableState { @@ -160,6 +161,7 @@ export const createThrottledStore = ( ): PrivateThrottledStore => { let pendingBroadcastNotification = false let pendingObservableNotifications: Set | undefined + let deferNotifications = false const onBroadcastNotify = () => { pendingBroadcastNotification = true @@ -210,6 +212,9 @@ export const createThrottledStore = ( const notifyAll = () => { try { + if (deferNotifications) { + return + } updateIsObserversNotifyInProgress(true) notifyObservers() updateIsSubscriptionNotifyInProgress(true) @@ -250,7 +255,8 @@ export const createThrottledStore = ( broadcastNotify: onBroadcastNotify, observableNotify: onObservableNotify, resetPendingNotifications: resetAllPendingNotifications, - hasPendingSubscribers: () => pendingBroadcastNotification + hasPendingSubscribers: () => pendingBroadcastNotification, + deferNotifications: (value: boolean) => {deferNotifications = value}, } resetAllPendingNotifications() diff --git a/testKit/index.tsx b/testKit/index.tsx index e896bfdb..5e337579 100644 --- a/testKit/index.tsx +++ b/testKit/index.tsx @@ -258,7 +258,8 @@ function createShell(host: AppHost): PrivateShell { clearCache: _.noop, getHostOptions: () => host.options, log: createShellLogger(host, entryPoint), - wrapWithShellRenderer: (component: JSX.Element) => component + wrapWithShellRenderer: (component: JSX.Element) => component, + deferSubscriberNotifications: _.identity, } } From b4549ad60a7f09d2b29a982bada564c341d6e730 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Wed, 27 Dec 2023 22:05:43 +0200 Subject: [PATCH 02/13] move to store --- src/API.ts | 10 +--------- src/appHost.tsx | 14 ++------------ src/throttledStore.tsx | 15 +++++++++++++-- testKit/index.tsx | 1 - 4 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/API.ts b/src/API.ts index cb40b8ac..f5b86a85 100644 --- a/src/API.ts +++ b/src/API.ts @@ -4,7 +4,7 @@ import { ThrottledStore } from './throttledStore' export { AppHostAPI } from './appHostServices' -export type ScopedStore = Pick, 'dispatch' | 'getState' | 'subscribe' | 'flush' | 'hasPendingSubscribers'> +export type ScopedStore = Pick, 'dispatch' | 'getState' | 'subscribe' | 'flush' | 'hasPendingSubscribers' | 'deferSubscriberNotifications'> export type ReactComponentContributor = (props?: TProps) => React.ReactNode export type ReducersMapObjectContributor = () => Redux.ReducersMapObject< TState, @@ -471,14 +471,6 @@ export interface Shell extends Pick & Partial)} memoizedFunction */ clearCache(memoizedFunction: Partial<_.MemoizedFunction> & Partial): void - /** - * Defer subscribers notification while performing a function - * - * @param func - */ - deferSubscriberNotifications( - func: () => T | Promise, - ): Promise } export interface PrivateShell extends Shell { diff --git a/src/appHost.tsx b/src/appHost.tsx index b52aea05..7b731adf 100644 --- a/src/appHost.tsx +++ b/src/appHost.tsx @@ -1058,6 +1058,7 @@ miss: ${memoizedWithMissHit.miss} }, flush: host.getStore().flush, hasPendingSubscribers: host.getStore().hasPendingSubscribers + deferSubscriberNotifications: host.getStore().deferSubscriberNotifications } }, @@ -1092,18 +1093,7 @@ miss: ${memoizedWithMissHit.miss} return }, - deferSubscriberNotifications: async (func) => { - try { - (host.getStore() as PrivateThrottledStore).deferNotifications(true); - const functionResult = await func(); - return functionResult; - - } - finally { - (host.getStore() as PrivateThrottledStore).deferNotifications(false); - host.getStore().flush(); - } - }, + } return shell diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index d3746bd8..7d68ee77 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -47,6 +47,7 @@ export interface StateContribution extends Store { hasPendingSubscribers(): boolean flush(): void + deferSubscriberNotifications(action: () => K | Promise): Promise } export interface PrivateThrottledStore extends ThrottledStore { @@ -55,7 +56,6 @@ export interface PrivateThrottledStore extends ThrottledStore { resetPendingNotifications(): void syncSubscribe(listener: () => void): Unsubscribe dispatchWithShell(shell: Shell): Dispatch - deferNotifications(flag: boolean): void } export interface PrivateObservableState extends ObservableState { @@ -256,7 +256,18 @@ export const createThrottledStore = ( observableNotify: onObservableNotify, resetPendingNotifications: resetAllPendingNotifications, hasPendingSubscribers: () => pendingBroadcastNotification, - deferNotifications: (value: boolean) => {deferNotifications = value}, + deferSubscriberNotifications: async (action) => { + try { + deferNotifications = true; + const functionResult = await action(); + return functionResult; + + } + finally { + deferNotifications = false; + flush(); + } + }, } resetAllPendingNotifications() diff --git a/testKit/index.tsx b/testKit/index.tsx index 5e337579..5840a822 100644 --- a/testKit/index.tsx +++ b/testKit/index.tsx @@ -259,7 +259,6 @@ function createShell(host: AppHost): PrivateShell { getHostOptions: () => host.options, log: createShellLogger(host, entryPoint), wrapWithShellRenderer: (component: JSX.Element) => component, - deferSubscriberNotifications: _.identity, } } From b1a9b53ef4b9bd5329741f152f50b4233db40058 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Thu, 28 Dec 2023 09:22:45 +0200 Subject: [PATCH 03/13] add support for nested defer notifications and fix lint --- src/API.ts | 5 ++++- src/appHost.tsx | 6 ++---- src/throttledStore.tsx | 21 +++++++++++---------- testKit/index.tsx | 2 +- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/API.ts b/src/API.ts index f5b86a85..8be0989a 100644 --- a/src/API.ts +++ b/src/API.ts @@ -4,7 +4,10 @@ import { ThrottledStore } from './throttledStore' export { AppHostAPI } from './appHostServices' -export type ScopedStore = Pick, 'dispatch' | 'getState' | 'subscribe' | 'flush' | 'hasPendingSubscribers' | 'deferSubscriberNotifications'> +export type ScopedStore = Pick< + ThrottledStore, + 'dispatch' | 'getState' | 'subscribe' | 'flush' | 'hasPendingSubscribers' | 'deferSubscriberNotifications' +> export type ReactComponentContributor = (props?: TProps) => React.ReactNode export type ReducersMapObjectContributor = () => Redux.ReducersMapObject< TState, diff --git a/src/appHost.tsx b/src/appHost.tsx index 7b731adf..a8efffc0 100644 --- a/src/appHost.tsx +++ b/src/appHost.tsx @@ -1057,7 +1057,7 @@ miss: ${memoizedWithMissHit.miss} return entireStoreState[shell.name] }, flush: host.getStore().flush, - hasPendingSubscribers: host.getStore().hasPendingSubscribers + hasPendingSubscribers: host.getStore().hasPendingSubscribers, deferSubscriberNotifications: host.getStore().deferSubscriberNotifications } }, @@ -1091,9 +1091,7 @@ miss: ${memoizedWithMissHit.miss} wrapWithShellRenderer(component): JSX.Element { return - }, - - + } } return shell diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index 7d68ee77..a65f8625 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -256,18 +256,19 @@ export const createThrottledStore = ( observableNotify: onObservableNotify, resetPendingNotifications: resetAllPendingNotifications, hasPendingSubscribers: () => pendingBroadcastNotification, - deferSubscriberNotifications: async (action) => { + deferSubscriberNotifications: async action => { + if (deferNotifications) { + return action() + } try { - deferNotifications = true; - const functionResult = await action(); - return functionResult; - + deferNotifications = true + const functionResult = await action() + return functionResult + } finally { + deferNotifications = false + flush() } - finally { - deferNotifications = false; - flush(); - } - }, + } } resetAllPendingNotifications() diff --git a/testKit/index.tsx b/testKit/index.tsx index 5840a822..e896bfdb 100644 --- a/testKit/index.tsx +++ b/testKit/index.tsx @@ -258,7 +258,7 @@ function createShell(host: AppHost): PrivateShell { clearCache: _.noop, getHostOptions: () => host.options, log: createShellLogger(host, entryPoint), - wrapWithShellRenderer: (component: JSX.Element) => component, + wrapWithShellRenderer: (component: JSX.Element) => component } } From 3088fd48c71029b68b2361c5dc768a9188b89f48 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Thu, 28 Dec 2023 12:54:52 +0200 Subject: [PATCH 04/13] add tests --- test/connectWithShell.spec.tsx | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/test/connectWithShell.spec.tsx b/test/connectWithShell.spec.tsx index 3f3e23cb..0c0d7cc3 100644 --- a/test/connectWithShell.spec.tsx +++ b/test/connectWithShell.spec.tsx @@ -589,6 +589,7 @@ describe('connectWithShell-useCases', () => { beforeEach(() => { renderSpy.mockClear() mapStateToPropsSpy.mockClear() + mountSpy.mockClear() }) it('should include observable state in store', () => { @@ -636,6 +637,81 @@ describe('connectWithShell-useCases', () => { expect(hasPendingSubscribers()).toBe(false) }) + it('should not notify subscribers when defering notifications', async () => { + const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI]) + const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp) + + const { testKit } = renderInShellContext() + if (!testKit) { + throw new Error('Connected component failed to render') + } + + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) + expect(renderSpy).toHaveBeenCalledTimes(1) + act(async () => { + await host.getStore().deferSubscriberNotifications(() => { + dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) + }) + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) + expect(renderSpy).toHaveBeenCalledTimes(2) + }) + }) + + it('should notify after defered notification action failed', () => { + const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI]) + const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp) + + const { testKit } = renderInShellContext() + if (!testKit) { + throw new Error('Connected component failed to render') + } + + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) + expect(renderSpy).toHaveBeenCalledTimes(1) + act(async () => { + await host.getStore().deferSubscriberNotifications(() => { + dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) + throw new Error('test error') + }) + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) + expect(renderSpy).toHaveBeenCalledTimes(2) + }) + }) + + it('should support nested defered notification actions', () => { + const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI]) + const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp) + + const { testKit } = renderInShellContext() + if (!testKit) { + throw new Error('Connected component failed to render') + } + + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) + expect(renderSpy).toHaveBeenCalledTimes(1) + act(async () => { + await host.getStore().deferSubscriberNotifications(async () => { + dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update from outer' }, host) + await host.getStore().deferSubscriberNotifications(() => { + dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update from inner' }, host) + }) + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) + expect(renderSpy).toHaveBeenCalledTimes(1) + }) + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update from inner') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) + expect(renderSpy).toHaveBeenCalledTimes(2) + }) + }) + 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) From c2a85e0d8a4b631a47b0865e8ef04ef80484ad80 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Thu, 28 Dec 2023 16:49:06 +0200 Subject: [PATCH 05/13] log error when flushing while defering notifications --- src/throttledStore.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index a65f8625..2fac1aab 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -235,6 +235,9 @@ export const createThrottledStore = ( }) const flush = () => { + if (deferNotifications) { + console.error('Cannot flush while notifications are deferred') + } cancelRender() notifyAll() } From 4e8e497db5e08115d8aee5b47a7b931c3f02a6c5 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Thu, 28 Dec 2023 16:49:20 +0200 Subject: [PATCH 06/13] fix tests --- test/connectWithShell.spec.tsx | 59 +++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/test/connectWithShell.spec.tsx b/test/connectWithShell.spec.tsx index 0c0d7cc3..1bcabc1b 100644 --- a/test/connectWithShell.spec.tsx +++ b/test/connectWithShell.spec.tsx @@ -649,19 +649,23 @@ describe('connectWithShell-useCases', () => { expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) - act(async () => { + + let valueOneWhileDefering + + await act(async () => { await host.getStore().deferSubscriberNotifications(() => { dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') - expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) + valueOneWhileDefering = testKit.root.findByProps({ className: 'ONE' }).children[0] }) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) - expect(renderSpy).toHaveBeenCalledTimes(2) }) + + expect(valueOneWhileDefering).toBe('init1') + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) + expect(renderSpy).toHaveBeenCalledTimes(2) }) - it('should notify after defered notification action failed', () => { + it('should notify after action failed while defering notifications', async () => { const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI]) const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp) @@ -673,18 +677,22 @@ describe('connectWithShell-useCases', () => { expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) - act(async () => { - await host.getStore().deferSubscriberNotifications(() => { - dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) - throw new Error('test error') + + try { + await act(async () => { + await host.getStore().deferSubscriberNotifications(() => { + dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) + throw new Error('test error') + }) }) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) - expect(renderSpy).toHaveBeenCalledTimes(2) - }) + } catch (e) {} + + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) + expect(renderSpy).toHaveBeenCalledTimes(2) }) - it('should support nested defered notification actions', () => { + it('should support nested defered notification actions', async () => { const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI]) const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp) @@ -696,20 +704,25 @@ describe('connectWithShell-useCases', () => { expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) - act(async () => { + let valueOneInOuterDeferNotifications + let valueOneInInnerDeferNotifications + + await act(async () => { await host.getStore().deferSubscriberNotifications(async () => { dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update from outer' }, host) await host.getStore().deferSubscriberNotifications(() => { dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update from inner' }, host) + valueOneInInnerDeferNotifications = testKit.root.findByProps({ className: 'ONE' }).children[0] }) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') - expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) - expect(renderSpy).toHaveBeenCalledTimes(1) + valueOneInOuterDeferNotifications = testKit.root.findByProps({ className: 'ONE' }).children[0] }) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update from inner') - expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) - expect(renderSpy).toHaveBeenCalledTimes(2) }) + + expect(valueOneInInnerDeferNotifications).toBe('init1') + expect(valueOneInOuterDeferNotifications).toBe('init1') + expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update from inner') + expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) + expect(renderSpy).toHaveBeenCalledTimes(2) }) it('should not mount connected component on props update', () => { From be6d624971e36fc4c02e9cb80fe9a3b55e10a4f8 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Sun, 31 Dec 2023 16:17:53 +0200 Subject: [PATCH 07/13] notify if needed instead of flush --- src/throttledStore.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index 2fac1aab..126efc0e 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -211,10 +211,12 @@ export const createThrottledStore = ( } const notifyAll = () => { + if (deferNotifications) { + updateIsObserversNotifyInProgress(true) + updateIsSubscriptionNotifyInProgress(true) + return + } try { - if (deferNotifications) { - return - } updateIsObserversNotifyInProgress(true) notifyObservers() updateIsSubscriptionNotifyInProgress(true) @@ -246,7 +248,10 @@ export const createThrottledStore = ( return store.dispatch(action) } - const toShellAction = (shell: Shell, action: T): T => ({ ...action, __shellName: shell.name }) + const toShellAction = (shell: Shell, action: T): T => ({ + ...action, + __shellName: shell.name + }) const result: PrivateThrottledStore = { ...store, @@ -269,7 +274,8 @@ export const createThrottledStore = ( return functionResult } finally { deferNotifications = false - flush() + notifyObservers() + notifySubscribers() } } } From 0dc28ca0ac0ed414924df4110ceb5048122e4803 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Sun, 31 Dec 2023 16:45:28 +0200 Subject: [PATCH 08/13] remove notify --- src/throttledStore.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index 126efc0e..1298c7c2 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -274,8 +274,6 @@ export const createThrottledStore = ( return functionResult } finally { deferNotifications = false - notifyObservers() - notifySubscribers() } } } From f9518c274d0b5ff6c59332263e0f98d721020942 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Sun, 31 Dec 2023 17:17:38 +0200 Subject: [PATCH 09/13] remove flush error --- src/throttledStore.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index 1298c7c2..beecda18 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -237,9 +237,6 @@ export const createThrottledStore = ( }) const flush = () => { - if (deferNotifications) { - console.error('Cannot flush while notifications are deferred') - } cancelRender() notifyAll() } From a3145e7c997eccf07614307e05d33cf5884d2d87 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Mon, 1 Jan 2024 10:26:12 +0200 Subject: [PATCH 10/13] notify after deferring notifications --- src/throttledStore.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index beecda18..55801a2a 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -271,6 +271,8 @@ export const createThrottledStore = ( return functionResult } finally { deferNotifications = false + notifyObservers() + notifySubscribers() } } } From c98ee0a6f13bb8d530941ecf2807bfbd5e707b62 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Mon, 1 Jan 2024 16:50:23 +0200 Subject: [PATCH 11/13] fix flush --- src/throttledStore.tsx | 31 +++++++++++++++++++++---------- test/connectWithShell.spec.tsx | 32 +++++++++++++++++++++++++++----- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index 55801a2a..1eaa393a 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -46,7 +46,7 @@ export interface StateContribution extends Store { hasPendingSubscribers(): boolean - flush(): void + flush(config?: { excecutionType: 'scheduled' | 'immediate' }): void deferSubscriberNotifications(action: () => K | Promise): Promise } @@ -162,6 +162,7 @@ export const createThrottledStore = ( let pendingBroadcastNotification = false let pendingObservableNotifications: Set | undefined let deferNotifications = false + let pendingFlush = false const onBroadcastNotify = () => { pendingBroadcastNotification = true @@ -211,11 +212,6 @@ export const createThrottledStore = ( } const notifyAll = () => { - if (deferNotifications) { - updateIsObserversNotifyInProgress(true) - updateIsSubscriptionNotifyInProgress(true) - return - } try { updateIsObserversNotifyInProgress(true) notifyObservers() @@ -228,7 +224,14 @@ export const createThrottledStore = ( } } - const notifyAllOnAnimationFrame = animationFrameRenderer(requestAnimationFrame, cancelAnimationFrame, notifyAll) + const scheduledNotifyAll = () => { + if (deferNotifications) { + return + } + notifyAll() + } + + const notifyAllOnAnimationFrame = animationFrameRenderer(requestAnimationFrame, cancelAnimationFrame, scheduledNotifyAll) let cancelRender = _.noop @@ -236,7 +239,11 @@ export const createThrottledStore = ( cancelRender = notifyAllOnAnimationFrame() }) - const flush = () => { + const flush = (config?: { excecutionType: 'scheduled' | 'immediate' }) => { + if (deferNotifications && config?.excecutionType !== 'immediate') { + pendingFlush = true + return + } cancelRender() notifyAll() } @@ -271,8 +278,12 @@ export const createThrottledStore = ( return functionResult } finally { deferNotifications = false - notifyObservers() - notifySubscribers() + if (pendingFlush) { + pendingFlush = false + flush() + } else { + notifyAll() + } } } } diff --git a/test/connectWithShell.spec.tsx b/test/connectWithShell.spec.tsx index 1bcabc1b..7f33372b 100644 --- a/test/connectWithShell.spec.tsx +++ b/test/connectWithShell.spec.tsx @@ -637,7 +637,7 @@ describe('connectWithShell-useCases', () => { expect(hasPendingSubscribers()).toBe(false) }) - it('should not notify subscribers when defering notifications', async () => { + 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) @@ -650,22 +650,22 @@ describe('connectWithShell-useCases', () => { expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) - let valueOneWhileDefering + let valueOneWhileDeferring await act(async () => { await host.getStore().deferSubscriberNotifications(() => { dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) - valueOneWhileDefering = testKit.root.findByProps({ className: 'ONE' }).children[0] + valueOneWhileDeferring = testKit.root.findByProps({ className: 'ONE' }).children[0] }) }) - expect(valueOneWhileDefering).toBe('init1') + expect(valueOneWhileDeferring).toBe('init1') expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) }) - it('should notify after action failed while defering notifications', async () => { + 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) @@ -692,6 +692,28 @@ describe('connectWithShell-useCases', () => { expect(renderSpy).toHaveBeenCalledTimes(2) }) + 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 { testKit } = renderInShellContext() + if (!testKit) { + throw new Error('Connected component failed to render') + } + + let valueOneWhileDeferring + + await host.getStore().deferSubscriberNotifications(() => { + act(() => { + host.getStore().dispatch({ type: 'SET_FIRST_STATE', value: 'update1' }) + host.getStore().flush({ excecutionType: 'immediate' }) + }) + valueOneWhileDeferring = testKit.root.findByProps({ className: 'ONE' }).children[0] + }) + + expect(valueOneWhileDeferring).toEqual('update1') + }) + it('should support nested defered notification actions', async () => { const { host, shell, renderInShellContext } = createMocks(entryPointWithState, [entryPointSecondStateWithAPI]) const ConnectedComp = connectWithShell(mapStateToProps, undefined, shell, { allowOutOfEntryPoint: true })(PureComp) From 35be76b5f218ab5a65dd84401a94a6801b00ed8b Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Tue, 2 Jan 2024 11:34:08 +0200 Subject: [PATCH 12/13] extract to function --- test/connectWithShell.spec.tsx | 75 ++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/test/connectWithShell.spec.tsx b/test/connectWithShell.spec.tsx index 7f33372b..9e127145 100644 --- a/test/connectWithShell.spec.tsx +++ b/test/connectWithShell.spec.tsx @@ -586,6 +586,9 @@ describe('connectWithShell-useCases', () => { } } + const getComponentValueByClassName = (className: string, testKit: ReactTestRenderer) => + testKit.root.findByProps({ className }).children[0] + beforeEach(() => { renderSpy.mockClear() mapStateToPropsSpy.mockClear() @@ -646,7 +649,7 @@ describe('connectWithShell-useCases', () => { throw new Error('Connected component failed to render') } - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(getComponentValueByClassName('ONE', testKit)).toBe('init1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) @@ -655,12 +658,12 @@ describe('connectWithShell-useCases', () => { await act(async () => { await host.getStore().deferSubscriberNotifications(() => { dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) - valueOneWhileDeferring = testKit.root.findByProps({ className: 'ONE' }).children[0] + valueOneWhileDeferring = getComponentValueByClassName('ONE', testKit) }) }) expect(valueOneWhileDeferring).toBe('init1') - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) }) @@ -674,7 +677,7 @@ describe('connectWithShell-useCases', () => { throw new Error('Connected component failed to render') } - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(getComponentValueByClassName('ONE', testKit)).toBe('init1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) @@ -687,7 +690,7 @@ describe('connectWithShell-useCases', () => { }) } catch (e) {} - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) }) @@ -708,7 +711,7 @@ describe('connectWithShell-useCases', () => { host.getStore().dispatch({ type: 'SET_FIRST_STATE', value: 'update1' }) host.getStore().flush({ excecutionType: 'immediate' }) }) - valueOneWhileDeferring = testKit.root.findByProps({ className: 'ONE' }).children[0] + valueOneWhileDeferring = getComponentValueByClassName('ONE', testKit) }) expect(valueOneWhileDeferring).toEqual('update1') @@ -723,7 +726,7 @@ describe('connectWithShell-useCases', () => { throw new Error('Connected component failed to render') } - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') + expect(getComponentValueByClassName('ONE', testKit)).toBe('init1') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) let valueOneInOuterDeferNotifications @@ -734,15 +737,15 @@ describe('connectWithShell-useCases', () => { dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update from outer' }, host) await host.getStore().deferSubscriberNotifications(() => { dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update from inner' }, host) - valueOneInInnerDeferNotifications = testKit.root.findByProps({ className: 'ONE' }).children[0] + valueOneInInnerDeferNotifications = getComponentValueByClassName('ONE', testKit) }) - valueOneInOuterDeferNotifications = testKit.root.findByProps({ className: 'ONE' }).children[0] + valueOneInOuterDeferNotifications = getComponentValueByClassName('ONE', testKit) }) }) expect(valueOneInInnerDeferNotifications).toBe('init1') expect(valueOneInOuterDeferNotifications).toBe('init1') - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update from inner') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update from inner') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) }) @@ -773,29 +776,29 @@ describe('connectWithShell-useCases', () => { throw new Error('Connected component failed to render') } - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') + expect(getComponentValueByClassName('ONE', testKit)).toBe('init1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) dispatchAndFlush({ type: 'SET_SECOND_STATE', value: 'update2' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('update2') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('update2') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(3) expect(renderSpy).toHaveBeenCalledTimes(3) dispatchAndFlush({ type: 'SOME_OTHER_ACTION' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('update2') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('update2') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(3) expect(renderSpy).toHaveBeenCalledTimes(3) }) @@ -812,23 +815,23 @@ describe('connectWithShell-useCases', () => { throw new Error('Connected component failed to render') } - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') + expect(getComponentValueByClassName('ONE', testKit)).toBe('init1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) // this should not notify the uninterested component dispatchAndFlush({ type: 'SET_FIRST_OBSERVABLE', value: 'update3' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) }) @@ -861,34 +864,34 @@ describe('connectWithShell-useCases', () => { throw new Error('Connected component failed to render') } - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('init1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') - expect(testKit.root.findByProps({ className: 'THREE' }).children[0]).toBe('init3') + expect(getComponentValueByClassName('ONE', testKit)).toBe('init1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') + expect(getComponentValueByClassName('THREE', testKit)).toBe('init3') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1) expect(renderSpy).toHaveBeenCalledTimes(1) dispatchAndFlush({ type: 'SET_FIRST_STATE', value: 'update1' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') - expect(testKit.root.findByProps({ className: 'THREE' }).children[0]).toBe('init3') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') + expect(getComponentValueByClassName('THREE', testKit)).toBe('init3') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2) expect(renderSpy).toHaveBeenCalledTimes(2) dispatchAndFlush({ type: 'SET_FIRST_OBSERVABLE', value: 'update3' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') - expect(testKit.root.findByProps({ className: 'THREE' }).children[0]).toBe('update3') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') + expect(getComponentValueByClassName('THREE', testKit)).toBe('update3') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(3) expect(renderSpy).toHaveBeenCalledTimes(3) dispatchAndFlush({ type: 'SOME_OTHER_ACTION' }, host) - expect(testKit.root.findByProps({ className: 'ONE' }).children[0]).toBe('update1') - expect(testKit.root.findByProps({ className: 'TWO' }).children[0]).toBe('init2') - expect(testKit.root.findByProps({ className: 'THREE' }).children[0]).toBe('update3') + expect(getComponentValueByClassName('ONE', testKit)).toBe('update1') + expect(getComponentValueByClassName('TWO', testKit)).toBe('init2') + expect(getComponentValueByClassName('THREE', testKit)).toBe('update3') expect(mapStateToPropsSpy).toHaveBeenCalledTimes(3) expect(renderSpy).toHaveBeenCalledTimes(3) }) From e3ca73913dee2b94ef3b532c0594f728cf555af1 Mon Sep 17 00:00:00 2001 From: ShirShintel Date: Thu, 4 Jan 2024 11:11:58 +0200 Subject: [PATCH 13/13] add default option to flush config --- src/throttledStore.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx index 1eaa393a..d9cf7ee4 100644 --- a/src/throttledStore.tsx +++ b/src/throttledStore.tsx @@ -46,7 +46,7 @@ export interface StateContribution extends Store { hasPendingSubscribers(): boolean - flush(config?: { excecutionType: 'scheduled' | 'immediate' }): void + flush(config?: { excecutionType: 'scheduled' | 'immediate' | 'default' }): void deferSubscriberNotifications(action: () => K | Promise): Promise } @@ -239,12 +239,14 @@ export const createThrottledStore = ( cancelRender = notifyAllOnAnimationFrame() }) - const flush = (config?: { excecutionType: 'scheduled' | 'immediate' }) => { - if (deferNotifications && config?.excecutionType !== 'immediate') { + const flush = (config = { excecutionType: 'default' }) => { + if (deferNotifications && config.excecutionType !== 'immediate') { pendingFlush = true return } - cancelRender() + if (config.excecutionType !== 'scheduled') { + cancelRender() + } notifyAll() }