Notify if needed before deferring subscriber notifications (#251)

* notify before defer * add test * only notify if needed * cleanup * fix typo * fix implementation * use notifyAll * rename * add test
wix-incubator · Mar 3, 2024 · d250474 · d250474
1 parent 77c3e14
commit d250474
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 12 deletions.
diff --git a/src/throttledStore.tsx b/src/throttledStore.tsx
@@ -161,7 +161,7 @@ export const createThrottledStore = (
 ): PrivateThrottledStore => {
     let pendingBroadcastNotification = false
     let pendingObservableNotifications: Set<AnyPrivateObservableState> | undefined
-    let deferNotifications = false
+    let isDeferrringNotifications = false
     let pendingFlush = false
 
     const onBroadcastNotify = () => {
@@ -225,7 +225,7 @@ export const createThrottledStore = (
     }
 
     const scheduledNotifyAll = () => {
-        if (deferNotifications) {
+        if (isDeferrringNotifications) {
             return
         }
         notifyAll()
@@ -240,7 +240,7 @@ export const createThrottledStore = (
     })
 
     const flush = (config = { excecutionType: 'default' }) => {
-        if (deferNotifications && config.excecutionType !== 'immediate') {
+        if (isDeferrringNotifications && config.excecutionType !== 'immediate') {
             pendingFlush = true
             return
         }
@@ -259,6 +259,15 @@ export const createThrottledStore = (
         __shellName: shell.name
     })
 
+    const executePendingActions = () => {
+        if (pendingFlush) {
+            pendingFlush = false
+            flush()
+        } else if (pendingBroadcastNotification || pendingObservableNotifications) {
+            notifyAll()
+        }
+    }
+
     const result: PrivateThrottledStore = {
         ...store,
         subscribe,
@@ -271,21 +280,17 @@ export const createThrottledStore = (
         resetPendingNotifications: resetAllPendingNotifications,
         hasPendingSubscribers: () => pendingBroadcastNotification,
         deferSubscriberNotifications: async action => {
-            if (deferNotifications) {
+            if (isDeferrringNotifications) {
                 return action()
             }
             try {
-                deferNotifications = true
+                executePendingActions()
+                isDeferrringNotifications = true
                 const functionResult = await action()
                 return functionResult
             } finally {
-                deferNotifications = false
-                if (pendingFlush) {
-                    pendingFlush = false
-                    flush()
-                } else {
-                    notifyAll()
-                }
+                isDeferrringNotifications = false
+                executePendingActions()
             }
         }
     }

diff --git a/test/connectWithShell.spec.tsx b/test/connectWithShell.spec.tsx
@@ -668,6 +668,44 @@ describe('connectWithShell-useCases', () => {
         expect(renderSpy).toHaveBeenCalledTimes(2)
     })
 
+    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 { testKit } = renderInShellContext(<ConnectedComp />)
+        if (!testKit) {
+            throw new Error('Connected component failed to render')
+        }
+
+        let hasPendingSubscribersWhileDeferring
+        await act(async () => {
+            host.getStore().dispatch({ type: 'SET_FIRST_STATE', value: 'update1' })
+            await host.getStore().deferSubscriberNotifications(() => {
+                hasPendingSubscribersWhileDeferring = shell.getStore().hasPendingSubscribers()
+            })
+        })
+
+        expect(hasPendingSubscribersWhileDeferring).toBe(false)
+    })
+
+    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 { testKit } = renderInShellContext(<ConnectedComp />)
+        if (!testKit) {
+            throw new Error('Connected component failed to render')
+        }
+        expect(mapStateToPropsSpy).toHaveBeenCalledTimes(1)
+
+        await act(async () => {
+            host.getStore().dispatch({ type: 'SET_FIRST_STATE', value: 'update1' })
+            await host.getStore().deferSubscriberNotifications(() => {
+                expect(mapStateToPropsSpy).toHaveBeenCalledTimes(2)
+            })
+        })
+    })
+
     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)