fix(focusManager): stop listening for focus events

[ilyasmez]

Why?

As discussed in #4797, focus event has many caveats, as it's triggered in the following scenarios:

• When interacting with an iframe.
• When opening/closing a browser dialog (alert, confirm).
• When moving between the page and DevTools.
• When opening/closing a file picker.

I think the original idea behind refetchOnWindowFocus was to refetch the queries when the user "comes back" to the page, and that's achievable using visibilitychange.

Before

You can see that the query is being refetched in many unexpected scenarios, while the user didn't leave the page.

rq-focus-fix-before.1.mp4

After

The query is refetched only when the user stop using the app, and comes back.

rq-focus-fix-after.1.mp4

What does this PR do

• Remove the focus event listener in focusManager.
• Update docs (also removed the old pitfalls and caveats, as they will be fixed with this change).
• Update tests (I went with consistency instead of trying to rewrite some of them).

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 52f4cda:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-vue-basic	Configuration

[TkDodo]

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

[TkDodo]

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

[DamianOsipiuk]

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

[TkDodo]

accidentally pressed request changes - i wanted to approve 😅

[DamianOsipiuk]

@TkDodo I believe we should add a note about this to the migration guide.

[TkDodo]

aah yes, forgot about that 👍 .
I'll do it on the v5 branch directly

[lbhbrave]

Hello guys. We have also encountered ux experience issues caused by focus recently and would like to know if this pull request can be merged into version V3. The risk is too high for us to perform a major version upgrade

[TkDodo]

if this pull request can be merged into version V3

it's a breaking change so no. also, it was merged to v5. seems like you haven't upgraded in over a year ...

[lbhbrave]

Yes. we are keeping using v3. And we fix this by git-patch. Maybe it's time to upgrade. Thx~ Dominik Dorfmeister ***@***.***> 于2023年7月15日周六 16:19写道：

…

if this pull request can be merged into version V3 it's a breaking change so no. also, it was merged to v5. seems like you haven't upgraded in over a year ... — Reply to this email directly, view it on GitHub <#4805 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACIBOGGUNCILAYE2DVUJ4YDXQJHAPANCNFSM6AAAAAATZUTNZQ> . You are receiving this because you commented.Message ID: ***@***.***>

[gabrielbrattgard]

@TkDodo @ilyasmez I might be wrong here but this only seems to work when switching between tabs and NOT between browser windows or returning from another desktop application. Is that intentional?

[TkDodo]

@gabrielbrattgard it does whatever the browser visibilitychange does so yes, that's an purpose. You can totally opt back into the focus event though with focusManager.setEventListener

[mrlubos]

This is great @ilyasmez!

[RiccardoRomagnoli]

@ilyasmez @TkDodo There is a case where without focus we can not archive the intended outcome.

With this change, now if you have two browser windows (one of them is our app), not maximised and you switch between one and the other (by click, by tab, eetc) the query will now not refetch anymore.

[ilyasmez]

There is a case where without focus we can not archive the intended outcome.

With this change, now if you have two browser windows (one of them is our app), not maximised and you switch between one and the other (by click, by tab, eetc) the query will now not refetch anymore.

@RiccardoRomagnoli, do you really want the page to refetch when the user interacts with it? This might result in content changing under their cursors (e.g. the item they wanted to click is not there anymore, or the position of the item changes). I believe that interacting with the page is already too late.
What you need is probably the broadcastQueryClient, it will update all the open windows immediately as mutations happen, which is a much superior user experience than waiting for the user to interact with the page, imo.

[HemalR]

There is a case where without focus we can not archive the intended outcome.
With this change, now if you have two browser windows (one of them is our app), not maximised and you switch between one and the other (by click, by tab, eetc) the query will now not refetch anymore.

@RiccardoRomagnoli, do you really want the page to refetch when the user interacts with it? This might result in content changing under their cursors (e.g. the item they wanted to click is not there anymore, or the position of the item changes). I believe that interacting with the page is already too late. What you need is probably the broadcastQueryClient, it will update all the open windows immediately as mutations happen, which is a much superior user experience than waiting for the user to interact with the page, imo.

This behaviour can be desirable in certain scenarios. E.g. A notification badge counter that can update when a user refocuses on to the browser window.

My workaround fix from the comments on this thread plus experimenting with other ways:

<script lang="ts">
	import { browser } from '$app/environment';
	import { QueryClient, QueryClientProvider, focusManager } from '@tanstack/svelte-query';
	import { onMount } from 'svelte';
	import '../app.css';

	let { children } = $props();

	const queryClient = new QueryClient({
		defaultOptions: {
			queries: {
				enabled: browser,
				refetchOnWindowFocus: true,
				staleTime: 0 // Set this to 0 to always consider data stale
			}
		}
	});

	onMount(() => {
		focusManager.setEventListener((handleFocus) => {
			const focus = () => {
                                 if (!document.hidden) {
				  console.log('Focus from parent');
				  handleFocus(true);
				  queryClient.refetchQueries(); // Without this, the above 'focus from parent' will be logged but queries will only refetch the first time the window is refocused from a different program. After that, data will not be refetched.
                                 }
			};
			window.addEventListener('visibilitychange', focus);
			window.addEventListener('focus', focus);
			return () => {
				window.removeEventListener('visibilitychange', focus);
				window.removeEventListener('focus', focus);
			};
		});
	});
</script>

<QueryClientProvider client={queryClient}>
	{@render children()}
</QueryClientProvider>

If anybody can explain the weird reason why the queries only refetch if manually triggered I'd love to know and improve what I'm doing here.