fix(core): onlineManager and focusManager should invoke listeners when a change occurred (#5713)

TkDodo · web-flow · commit 5322ede0a470 · 2023-07-14T16:44:27.000+02:00
This is only relevant when a custom online / focus boolean has been set; in those cases, we previously only informed listeners if the value was truthy, but not if it changed back to false or undefined again

With this fix, the managers inform the listeners whenever something has changed, regardless of which direction the change was
diff --git a/packages/query-core/src/focusManager.ts b/packages/query-core/src/focusManager.ts
@@ -58,9 +58,9 @@ export class FocusManager extends Subscribable {
   }
 
   setFocused(focused?: boolean): void {
-    this.focused = focused
-
-    if (focused) {
+    const changed = this.focused !== focused
+    if (changed) {
+      this.focused = focused
       this.onFocus()
     }
   }
diff --git a/packages/query-core/src/onlineManager.ts b/packages/query-core/src/onlineManager.ts
@@ -63,9 +63,10 @@ export class OnlineManager extends Subscribable {
   }
 
   setOnline(online?: boolean): void {
-    this.online = online
+    const changed = this.online !== online
 
-    if (online) {
+    if (changed) {
+      this.online = online
       this.onOnline()
     }
   }
diff --git a/packages/query-core/src/tests/focusManager.test.tsx b/packages/query-core/src/tests/focusManager.test.tsx
@@ -38,20 +38,6 @@ describe('focusManager', () => {
     expect(focusManager.isFocused()).toBeTruthy()
   })
 
-  it('should not notify listeners on focus if already focused', async () => {
-    const subscriptionSpy = jest.fn()
-    const unsubscribe = focusManager.subscribe(subscriptionSpy)
-
-    focusManager.setFocused(true)
-    expect(subscriptionSpy).toHaveBeenCalledTimes(1)
-    subscriptionSpy.mockReset()
-
-    focusManager.setFocused(false)
-    expect(subscriptionSpy).toHaveBeenCalledTimes(0)
-
-    unsubscribe()
-  })
-
   it('should return true for isFocused if document is undefined', async () => {
     const { document } = globalThis
 
@@ -152,4 +138,25 @@ describe('focusManager', () => {
 
     unsubscribe2()
   })
+
+  test('should call listeners when setFocused is called', () => {
+    const listener = jest.fn()
+
+    focusManager.subscribe(listener)
+
+    focusManager.setFocused(true)
+    focusManager.setFocused(true)
+
+    expect(listener).toHaveBeenCalledTimes(1)
+
+    focusManager.setFocused(false)
+    focusManager.setFocused(false)
+
+    expect(listener).toHaveBeenCalledTimes(2)
+
+    focusManager.setFocused(undefined)
+    focusManager.setFocused(undefined)
+
+    expect(listener).toHaveBeenCalledTimes(3)
+  })
 })
diff --git a/packages/query-core/src/tests/onlineManager.test.tsx b/packages/query-core/src/tests/onlineManager.test.tsx
@@ -145,4 +145,25 @@ describe('onlineManager', () => {
 
     unsubscribe2()
   })
+
+  test('should call listeners when setOnline is called', () => {
+    const listener = jest.fn()
+
+    onlineManager.subscribe(listener)
+
+    onlineManager.setOnline(true)
+    onlineManager.setOnline(true)
+
+    expect(listener).toHaveBeenCalledTimes(1)
+
+    onlineManager.setOnline(false)
+    onlineManager.setOnline(false)
+
+    expect(listener).toHaveBeenCalledTimes(2)
+
+    onlineManager.setOnline(undefined)
+    onlineManager.setOnline(undefined)
+
+    expect(listener).toHaveBeenCalledTimes(3)
+  })
 })

Original file line number	Diff line number	Diff line change
`@@ -58,9 +58,9 @@ export class FocusManager extends Subscribable {`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`setFocused(focused?: boolean): void {`
`61`		`- this.focused = focused`
`62`		`-`
`63`		`- if (focused) {`
	`61`	`+ const changed = this.focused !== focused`
	`62`	`+ if (changed) {`
	`63`	`+ this.focused = focused`
`64`	`64`	`this.onFocus()`
`65`	`65`	`}`
`66`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -63,9 +63,10 @@ export class OnlineManager extends Subscribable {`
`63`	`63`	`}`
`64`	`64`
`65`	`65`	`setOnline(online?: boolean): void {`
`66`		`- this.online = online`
	`66`	`+ const changed = this.online !== online`
`67`	`67`
`68`		`- if (online) {`
	`68`	`+ if (changed) {`
	`69`	`+ this.online = online`
`69`	`70`	`this.onOnline()`
`70`	`71`	`}`
`71`	`72`	`}`