Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(focusManager): stop listening for focus events #4805

Merged
merged 1 commit into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions docs/react/guides/important-defaults.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ Out of the box, TanStack Query is configured with **aggressive but sane** defaul
- The network is reconnected
- The query is optionally configured with a refetch interval

If you see a refetch that you are not expecting, it is likely because you just focused the window and TanStack Query is doing a [`refetchOnWindowFocus`](../guides/window-focus-refetching). During development, this will probably be triggered more frequently, especially because focusing between the Browser DevTools and your app will also cause a fetch, so be aware of that.

Comment on lines -18 to -19
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow this is great! I didn't know that it would also fix this issue ❤️

> To change this functionality, you can use options like `refetchOnMount`, `refetchOnWindowFocus`, `refetchOnReconnect` and `refetchInterval`.
- Query results that have no more active instances of `useQuery`, `useInfiniteQuery` or query observers are labeled as "inactive" and remain in the cache in case they are used again at a later time.
Expand Down
27 changes: 3 additions & 24 deletions docs/react/guides/window-focus-refetching.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,36 +48,19 @@ In rare circumstances, you may want to manage your own window focus events that

```tsx
focusManager.setEventListener((handleFocus) => {
// Listen to visibilitychange and focus
// Listen to visibilitychange
if (typeof window !== 'undefined' && window.addEventListener) {
window.addEventListener('visibilitychange', handleFocus, false)
window.addEventListener('focus', handleFocus, false)
}

return () => {
// Be sure to unsubscribe if a new handler is set
window.removeEventListener('visibilitychange', handleFocus)
window.removeEventListener('focus', handleFocus)
}
})
```

[//]: # 'Example3'

## Ignoring Iframe Focus Events

A great use-case for replacing the focus handler is that of iframe events. Iframes present problems with detecting window focus by both double-firing events and also firing false-positive events when focusing or using iframes within your app. If you experience this, you should use an event handler that ignores these events as much as possible. I recommend [this one](https://gist.github.com/tannerlinsley/1d3a2122332107fcd8c9cc379be10d88)! It can be set up in the following way:

[//]: # 'Example4'

```tsx
import { focusManager } from '@tanstack/react-query'
import onWindowFocus from './onWindowFocus' // The gist above

focusManager.setEventListener(onWindowFocus) // Boom!
```

[//]: # 'Example4'
[//]: # 'ReactNative'

## Managing Focus in React Native
Expand Down Expand Up @@ -105,7 +88,7 @@ useEffect(() => {

## Managing focus state

[//]: # 'Example5'
[//]: # 'Example4'
Comment on lines -108 to +91
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should switch to named examples @DamianOsipiuk 😅. I see the vue-query docs only overwrite/use the 'Example' slot, but this could become tedious with numbers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you know naming things is hard, so i just went with numbers 🙈
But yeah, probably we should name them properly.


```tsx
import { focusManager } from '@tanstack/react-query'
Expand All @@ -117,8 +100,4 @@ focusManager.setFocused(true)
focusManager.setFocused(undefined)
```

[//]: # 'Example5'

## Pitfalls & Caveats

Some browser internal dialogue windows, such as spawned by `alert()` or file upload dialogues (as created by `<input type="file" />`) might also trigger focus refetching after they close. This can result in unwanted side effects, as the refetching might trigger component unmounts or remounts before your file upload handler is executed. See [this issue on GitHub](https://github.com/tannerlinsley/react-query/issues/2960) for background and possible workarounds.
[//]: # 'Example4'
6 changes: 2 additions & 4 deletions docs/react/reference/focusManager.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,15 @@ Its available methods are:
```tsx
import { focusManager } from '@tanstack/react-query'

focusManager.setEventListener(handleFocus => {
// Listen to visibilitychange and focus
focusManager.setEventListener((handleFocus) => {
// Listen to visibilitychange
if (typeof window !== 'undefined' && window.addEventListener) {
window.addEventListener('visibilitychange', handleFocus, false)
window.addEventListener('focus', handleFocus, false)
}

return () => {
// Be sure to unsubscribe if a new handler is set
window.removeEventListener('visibilitychange', handleFocus)
window.removeEventListener('focus', handleFocus)
}
})
```
Expand Down
4 changes: 1 addition & 3 deletions packages/query-core/src/focusManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ export class FocusManager extends Subscribable {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (!isServer && window.addEventListener) {
const listener = () => onFocus()
// Listen to visibillitychange and focus
// Listen to visibilitychange
window.addEventListener('visibilitychange', listener, false)
window.addEventListener('focus', listener, false)

return () => {
// Be sure to unsubscribe if a new handler is set
window.removeEventListener('visibilitychange', listener)
window.removeEventListener('focus', listener)
}
}
return
Expand Down
34 changes: 13 additions & 21 deletions packages/query-core/src/tests/focusManager.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,31 +87,23 @@ describe('focusManager', () => {
})

it('should replace default window listener when a new event listener is set', async () => {
const addEventListenerSpy = jest.spyOn(
globalThis.window,
'addEventListener',
)
const unsubscribeSpy = jest.fn().mockImplementation(() => undefined)
const handlerSpy = jest.fn().mockImplementation(() => unsubscribeSpy)

const removeEventListenerSpy = jest.spyOn(
globalThis.window,
'removeEventListener',
)
focusManager.setEventListener(() => handlerSpy())

// Should set the default event listener with window event listeners
const unsubscribe = focusManager.subscribe(() => undefined)
expect(addEventListenerSpy).toHaveBeenCalledTimes(2)

// Should replace the window default event listener by a new one
// and it should call window.removeEventListener twice
focusManager.setEventListener(() => {
return () => void 0
})

expect(removeEventListenerSpy).toHaveBeenCalledTimes(2)
// Should call the custom event once
expect(handlerSpy).toHaveBeenCalledTimes(1)

unsubscribe()
addEventListenerSpy.mockRestore()
removeEventListenerSpy.mockRestore()

// Should unsubscribe our event event
expect(unsubscribeSpy).toHaveBeenCalledTimes(1)

handlerSpy.mockRestore()
unsubscribeSpy.mockRestore()
})

test('should call removeEventListener when last listener unsubscribes', () => {
Expand All @@ -127,12 +119,12 @@ describe('focusManager', () => {

const unsubscribe1 = focusManager.subscribe(() => undefined)
const unsubscribe2 = focusManager.subscribe(() => undefined)
expect(addEventListenerSpy).toHaveBeenCalledTimes(2) // visibilitychange + focus
expect(addEventListenerSpy).toHaveBeenCalledTimes(1) // visibilitychange event

unsubscribe1()
expect(removeEventListenerSpy).toHaveBeenCalledTimes(0)
unsubscribe2()
expect(removeEventListenerSpy).toHaveBeenCalledTimes(2) // visibilitychange + focus
expect(removeEventListenerSpy).toHaveBeenCalledTimes(1) // visibilitychange event
})

test('should keep setup function even if last listener unsubscribes', () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/query-core/src/tests/query.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('query', () => {

// Reset visibilityState to original value
visibilityMock.mockRestore()
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

// There should not be a result yet
expect(result).toBeUndefined()
Expand Down Expand Up @@ -181,7 +181,6 @@ describe('query', () => {
} finally {
// Reset visibilityState to original value
visibilityMock.mockRestore()
window.dispatchEvent(new FocusEvent('focus'))
}
})

Expand Down
22 changes: 11 additions & 11 deletions packages/react-query/src/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2608,7 +2608,7 @@ describe('useQuery', () => {
await waitFor(() => rendered.getByText('default'))

act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

expect(queryFn).not.toHaveBeenCalled()
Expand All @@ -2635,7 +2635,7 @@ describe('useQuery', () => {
await sleep(10)

act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

await sleep(10)
Expand Down Expand Up @@ -2666,7 +2666,7 @@ describe('useQuery', () => {
await sleep(10)

act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

await sleep(10)
Expand Down Expand Up @@ -2697,7 +2697,7 @@ describe('useQuery', () => {
await sleep(10)

act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

await sleep(10)
Expand Down Expand Up @@ -2732,7 +2732,7 @@ describe('useQuery', () => {
await sleep(20)

act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

await sleep(20)
Expand Down Expand Up @@ -2774,7 +2774,7 @@ describe('useQuery', () => {
expect(states[1]).toMatchObject({ data: 0, isFetching: false })

act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

await rendered.findByText('data: 1')
Expand All @@ -2786,7 +2786,7 @@ describe('useQuery', () => {
expect(states[3]).toMatchObject({ data: 1, isFetching: false })

act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

await sleep(20)
Expand Down Expand Up @@ -3519,7 +3519,7 @@ describe('useQuery', () => {
act(() => {
// reset visibilityState to original value
visibilityMock.mockRestore()
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

// Wait for the final result
Expand Down Expand Up @@ -3601,7 +3601,7 @@ describe('useQuery', () => {
act(() => {
// reset visibilityState to original value
visibilityMock.mockRestore()
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

await waitFor(() => expect(states.length).toBe(4))
Expand Down Expand Up @@ -5327,7 +5327,7 @@ describe('useQuery', () => {
rendered.getByText('status: success, fetchStatus: paused'),
)

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
await sleep(15)

await waitFor(() =>
Expand Down Expand Up @@ -5493,7 +5493,7 @@ describe('useQuery', () => {

// triggers a second pause
act(() => {
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
})

onlineMock.mockReturnValue(true)
Expand Down
20 changes: 10 additions & 10 deletions packages/solid-query/src/__tests__/createQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2474,7 +2474,7 @@ describe('createQuery', () => {

await waitFor(() => screen.getByText('default'))

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

expect(queryFn).not.toHaveBeenCalled()
})
Expand Down Expand Up @@ -2505,7 +2505,7 @@ describe('createQuery', () => {

await sleep(10)

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

await sleep(10)

Expand Down Expand Up @@ -2540,7 +2540,7 @@ describe('createQuery', () => {

await sleep(10)

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

await sleep(10)

Expand Down Expand Up @@ -2575,7 +2575,7 @@ describe('createQuery', () => {

await sleep(10)

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

await sleep(10)

Expand Down Expand Up @@ -2613,7 +2613,7 @@ describe('createQuery', () => {

await sleep(20)

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

await sleep(20)

Expand Down Expand Up @@ -2658,7 +2658,7 @@ describe('createQuery', () => {
expect(states[0]).toMatchObject({ data: undefined, isFetching: true })
expect(states[1]).toMatchObject({ data: 0, isFetching: false })

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

await screen.findByText('data: 1')

Expand Down Expand Up @@ -3469,7 +3469,7 @@ describe('createQuery', () => {
await waitFor(() => screen.getByText('failureReason fetching error 1'))

visibilityMock.mockRestore()
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

// Wait for the final result
await waitFor(() => screen.getByText('failureCount 4'))
Expand Down Expand Up @@ -3564,7 +3564,7 @@ describe('createQuery', () => {

// reset visibilityState to original value
visibilityMock.mockRestore()
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

await waitFor(() => expect(states.length).toBe(4))

Expand Down Expand Up @@ -5367,7 +5367,7 @@ describe('createQuery', () => {
screen.getByText('status: success, fetchStatus: paused'),
)

window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))
await sleep(15)

await waitFor(() =>
Expand Down Expand Up @@ -5544,7 +5544,7 @@ describe('createQuery', () => {
)

// triggers a second pause
window.dispatchEvent(new FocusEvent('focus'))
window.dispatchEvent(new Event('visibilitychange'))

onlineMock.mockReturnValue(true)
window.dispatchEvent(new Event('online'))
Expand Down