Skip to content

Commit

Permalink
fix(core): onlineManager and focusManager should invoke listeners whe…
Browse files Browse the repository at this point in the history
…n a change occurred (#5713)

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
  • Loading branch information
TkDodo authored Jul 14, 2023
1 parent c331b1f commit 5322ede
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 19 deletions.
6 changes: 3 additions & 3 deletions packages/query-core/src/focusManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand Down
5 changes: 3 additions & 2 deletions packages/query-core/src/onlineManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand Down
35 changes: 21 additions & 14 deletions packages/query-core/src/tests/focusManager.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
})
})
21 changes: 21 additions & 0 deletions packages/query-core/src/tests/onlineManager.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})

0 comments on commit 5322ede

Please sign in to comment.