SvelteStack / svelte-query

Performant and powerful remote data synchronization for Svelte
https://sveltequery.vercel.app
MIT License
814 stars 31 forks source link

Using `refetchOnWindowFocus` when I change it to false it fetches one more time! #48

Closed frederikhors closed 2 years ago

frederikhors commented 3 years ago

I'm using this code to query my players:

export let refetchEnabled: boolean;

$: players = useQuery("/players",
  async () => await getPlayers(),
  { refetchOnWindowFocus: refetchEnabled }
);

but in this way when I switch refetchEnabled to false it refetches my players one more time before stopping.

Am I wrong?

dimfeld commented 2 years ago

Does this work better for you? Don't know for sure that it will solve your issue but it avoids recreating the entire query when refetchEnabled changes.

export let refetchEnabled: boolean;

let players = useQuery("/players",
  async () => await getPlayers(),
  { refetchOnWindowFocus: refetchEnabled }
);

$: players.updateOptions({ refetchOnWindowFocus: refetchEnabled });
frederikhors commented 2 years ago

It works! Thanks!

frederikhors commented 2 years ago

The is no mention in docs about: updateOptions(), or am I wrong?

frederikhors commented 2 years ago

Can I ask you what's the difference between these code versions, given this one:

<script lang="ts">
    import { useQuery } from '@sveltestack/svelte-query';

    import { Condition } from '$lib/common';
    import { service, playerListKey } from '$lib/usecases/player';

    const PAGINATION = {
        DEFAULT: {
            after: '',
            before: '',
            first: 15,
            last: 0,
            orderBy: [{ field: 'id', desc: true }]
        }
    };

    export let refetchEnabled = false;
    export let condition = Condition.create();
    export let pagination = PAGINATION.DEFAULT;

    $: players = useQuery(
        [playerListKey, pagination, condition],
        async () => await service.playerList({ pagination, condition }).response,
        { refetchOnWindowFocus: refetchEnabled, enabled: refetchEnabled }
    );

    function previousPage() {
        pagination = {
            ...pagination,
            after: '',
            before: $players.data.pageInfo.startCursor,
            first: 0,
            last: PAGINATION.DEFAULT.first
        };
    }

    function nextPage() {
        pagination = {
            ...pagination,
            after: $players.data.pageInfo.endCursor,
            before: '',
            first: PAGINATION.DEFAULT.first,
            last: 0
        };
    }
</script>

<div>
    <button on:click={previousPage}>Next</button>
    <button on:click={nextPage}>Previous</button>
</div>

{#if $players.isLoading}
    Loading...
{:else if $players.isError}
    $players.error
{:else if $players.data}
    <ul>
        {#each $players.data.players as player (player.id)}
            <li><span>{player.name}</span></li>
        {:else}
            No players...
        {/each}
    </ul>
{/if}

So what's the difference between the first version:

$: players = useQuery(
  [playerListKey, pagination, condition],
  async () => await service.playerList({ pagination, condition }).response,
  { refetchOnWindowFocus: refetchEnabled }
);

the second one (doesn't work):

const playersQueryFn = async (pagination: Pagination, condition: Condition) =>
  await service.playerList({ pagination, condition }).response;

const players = useQuery([playerListKey, pagination, condition], () =>
  playersQueryFn(pagination, condition)
);

$: players.updateOptions({
  refetchOnWindowFocus: refetchEnabled,
  queryKey: [playerListKey, pagination, condition],
  queryFn: () => playersQueryFn(pagination, condition),
});

and the third one (doesn't work)?:

const playersQueryFn = async (pagination: Pagination, condition: Condition) =>
  await service.playerList({ pagination, condition }).response;

const players = useQuery([playerListKey, pagination, condition], () =>
  playersQueryFn(pagination, condition)
);

$: players.updateOptions({ refetchOnWindowFocus: refetchEnabled });
$: players.updateOptions({ queryKey: [playerListKey, pagination, condition] });
$: players.updateOptions({ queryFn: () => playersQueryFn(pagination, condition) });

Is there any performances difference?

You said for your code:

it avoids recreating the entire query when refetchEnabled changes

Is this a performance problem?

Is using many reactive variables the same as one query only in terms of performances?

Do you know how can we measure difference?

dimfeld commented 2 years ago

The is no mention in docs about: updateOptions(), or am I wrong?

Not that I've seen. I just found it in the source a while ago when I was looking for similar functionality.

You said for your code:

it avoids recreating the entire query when refetchEnabled changes

Is this a performance problem?

Not really, but my supposition was that recreating the query is causing it to run the fetch again when you didn't want it to.

As for the other questions, I think if you want to change the query key then you do have to actually recreate the whole query object.

frederikhors commented 2 years ago

if you want to change the query key then you do have to actually recreate the whole query object

@amen-souissi can you please help here? Svelte newbie here. 😄

@amen-souissi are you still interested in keeping the project and helping people use it? I sincerely ask just to understand for the future, no criticism between the lines... 😃

SomaticIT commented 2 years ago

useQuery should always be stored in a const. There is a bug in the current version when you update the queryKey using updateOptions.

This will be fixed in v1.5.0 (released today).

I closed this because it's mostly a duplicate of #52.