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

The `focus` event had many caveats (as discussed in #4797), so we now listen only for `visibilitychange` event.

Author: ilyasmez

docs/react/guides/important-defaults.md

@@ -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.
-
 > 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.

docs/react/guides/window-focus-refetching.md

@@ -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
@@ -105,7 +88,7 @@ useEffect(() => {
 
 ## Managing focus state
 
-[//]: # 'Example5'
+[//]: # 'Example4'
 
 ```tsx
 import { focusManager } from '@tanstack/react-query'
@@ -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'

docs/react/reference/focusManager.md

@@ -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)
   }
 })
 ```

packages/query-core/src/focusManager.ts

@@ -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

packages/query-core/src/tests/focusManager.test.tsx

@@ -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', () => {
@@ -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', () => {

packages/query-core/src/tests/query.test.tsx

@@ -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()
@@ -181,7 +181,6 @@ describe('query', () => {
     } finally {
       // Reset visibilityState to original value
       visibilityMock.mockRestore()
-      window.dispatchEvent(new FocusEvent('focus'))
     }
   })
 

packages/react-query/src/__tests__/useQuery.test.tsx

@@ -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()
@@ -2635,7 +2635,7 @@ describe('useQuery', () => {
     await sleep(10)
 
     act(() => {
-      window.dispatchEvent(new FocusEvent('focus'))
+      window.dispatchEvent(new Event('visibilitychange'))
     })
 
     await sleep(10)
@@ -2666,7 +2666,7 @@ describe('useQuery', () => {
     await sleep(10)
 
     act(() => {
-      window.dispatchEvent(new FocusEvent('focus'))
+      window.dispatchEvent(new Event('visibilitychange'))
     })
 
     await sleep(10)
@@ -2697,7 +2697,7 @@ describe('useQuery', () => {
     await sleep(10)
 
     act(() => {
-      window.dispatchEvent(new FocusEvent('focus'))
+      window.dispatchEvent(new Event('visibilitychange'))
     })
 
     await sleep(10)
@@ -2732,7 +2732,7 @@ describe('useQuery', () => {
     await sleep(20)
 
     act(() => {
-      window.dispatchEvent(new FocusEvent('focus'))
+      window.dispatchEvent(new Event('visibilitychange'))
     })
 
     await sleep(20)
@@ -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')
@@ -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)
@@ -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
@@ -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))
@@ -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(() =>
@@ -5493,7 +5493,7 @@ describe('useQuery', () => {
 
       // triggers a second pause
       act(() => {
-        window.dispatchEvent(new FocusEvent('focus'))
+        window.dispatchEvent(new Event('visibilitychange'))
       })
 
       onlineMock.mockReturnValue(true)

packages/solid-query/src/__tests__/createQuery.test.tsx

@@ -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()
   })
@@ -2505,7 +2505,7 @@ describe('createQuery', () => {
 
     await sleep(10)
 
-    window.dispatchEvent(new FocusEvent('focus'))
+    window.dispatchEvent(new Event('visibilitychange'))
 
     await sleep(10)
 
@@ -2540,7 +2540,7 @@ describe('createQuery', () => {
 
     await sleep(10)
 
-    window.dispatchEvent(new FocusEvent('focus'))
+    window.dispatchEvent(new Event('visibilitychange'))
 
     await sleep(10)
 
@@ -2575,7 +2575,7 @@ describe('createQuery', () => {
 
     await sleep(10)
 
-    window.dispatchEvent(new FocusEvent('focus'))
+    window.dispatchEvent(new Event('visibilitychange'))
 
     await sleep(10)
 
@@ -2613,7 +2613,7 @@ describe('createQuery', () => {
 
     await sleep(20)
 
-    window.dispatchEvent(new FocusEvent('focus'))
+    window.dispatchEvent(new Event('visibilitychange'))
 
     await sleep(20)
 
@@ -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')
 
@@ -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'))
@@ -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))
 
@@ -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(() =>
@@ -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'))