TanStack / query

🤖 Powerful asynchronous state management, server-state utilities and data fetching for the web. TS/JS, React Query, Solid Query, Svelte Query and Vue Query.
https://tanstack.com/query
MIT License
41.43k stars 2.81k forks source link

[svelte] "placeholderData: keepPreviousData" doesn't work on reactive queries #5913

Closed HugeLetters closed 6 months ago

HugeLetters commented 12 months ago

Describe the bug

placeholderData: keepPreviousData doesn't work correctly with reactive queries. It still shows undefined when changing keys.

Your minimal, reproducible example

https://stackblitz.com/edit/sveltejs-kit-template-default-fcwhut?file=src%2Froutes%2F%2Bpage.svelte

Steps to reproduce

<script lang="ts">
import { createQuery, keepPreviousData } from '@tanstack/svelte-query'; 
// "@tanstack/svelte-query": "5.0.0-beta.20"

let post: string;
$: query = createQuery({
    queryKey: [post],
    queryFn: () => fetch(`https://jsonplaceholder.typicode.com/posts/${post}`).then((x) => x.text()),
    placeholderData: keepPreviousData
});
</script>
<input bind:value={post}/>
<p>{$query.data}</p>

Expected behavior

I expect to see a previous post until a new post is successfully fetched.

How often does this bug happen?

Every time

Screenshots or Videos

https://github.com/TanStack/query/assets/119697239/49fcff5c-de39-4b30-bea5-df764b899250

Platform

Tanstack Query adapter

svelte-query

TanStack Query version

5.0.0-beta.20

TypeScript version

5.2.2

Additional context

Perhaps I'm just using this wrong - I assume query gets recreated on each post update, and also the keys update and that's why svelte-query cant' really understand where to get the previous data from?

TkDodo commented 12 months ago

@lachlancollins FYI

james-kaguru commented 12 months ago

could you try this

$: query = createQuery({ queryKey: [], queryFn: () => fetch(https://jsonplaceholder.typicode.com/posts/${post}).then((x) => x.text()), keepPreviousData: true });

setting the query key to be post will cause the the query to be re-evaluated everytime the post changes and cause the query to "restart" instead of picking up from where it left off..so just remove the reactive values like the post id from the query key and just place the reactive values on the url.

HugeLetters commented 12 months ago

could you try this

$: query = createQuery({ queryKey: [], queryFn: () => fetch(https://jsonplaceholder.typicode.com/posts/${post}).then((x) => x.text()), keepPreviousData: true });

setting the query key to be post will cause the the query to be re-evaluated everytime the post changes and cause the query to "restart" instead of picking up from where it left off..so just remove the reactive values like the post id from the query key and just place the reactive values on the url.

Having no key would mean there would be no cache. Ideal behavior would be

fetching post 1 -> displays undefined
fetched post 1 -> displays post 1
-
fetching post 2 -> displays post 1  from prev data
fetched post 2 -> displays post 2
-
fetching post 3 -> displays post 2 from prev data
fetched post 3 -> displays post 3
-
fetching post 1 -> displays post 1 from cache
fetched post 1 -> displays post 1
sefirosweb commented 10 months ago

@HugeLetters i'm not sure if i have same issue has you

keepPreviusData is getting last "queryFn" executed instead the last queryKey are used,

I'n my case i have executed url with params in querykey:

fetching post 1 -> displays undefined fetched post 1 -> displays post 1

fetching post 2 -> displays post 1 from prev data fetched post 2 -> displays post 2

.. return to post 1 fetch post 1 again ( now are in cache shows inmedatly post 1) -> displays post 1

fetch post 3 -> DISPLAY POST 2 fetched post 3 -> displays post 3

I updated to latest version to V5

frederikhors commented 10 months ago

Hi guys. I think the issue you're having is explained in https://github.com/TanStack/query/pull/5682. Use stores instead of reactive query declaration and let me know. See new svelte doc for V5 too.

HugeLetters commented 10 months ago

Hi guys. I think the issue you're having is explained in #5682. Use stores instead of reactive query declaration and let me know. See new svelte doc for V5 too.

@frederikhors Using a store works but not everything can be a store.

When I bind a local state to some input and base my query on that - using a writable store instead of let feels wrong but at least works and can be done. But in SvelteKit, say, if I have some data which get's passed in from load function which I then access with export let data - I don't know any way to make some value depend on it without a reactive statement. You can't create a derived store from a let variable as far as I know.

My point being that I think svelte-query should work with reactive statements too, it's pretty standard in Svelte - the fact that it recreates the whole createQuery instead of just its options shouldn't be an issue, that's already what React does and it works fine.

Please don't take my comment as "fix this immediately", I'm just giving my thoughts on how I think this should work and why I think that is. Runes in Svelte5 probably might fix this since stores/let vars are gonna be just one $state rune - that's also probably worth considering.

MoritzKronberger commented 10 months ago

I know it's a hack and it might not work for v5 but if you're already using custom methods to create your queries, you can keep previous data with currying.

Here is a minimal (pseudo-code) example, just make sure your version of createReactiveQuery is called for every reactive query instance to keep seperate previousData for all instances:

function createReactiveQuery (key, query) {
    // Keep track of previous data
    let previousData

    return (input) => {
        const queryStore = createQuery({
            queryKey: [key, input],
            queryFn: (context) => query(context.queryKey[1]),
            // Return previous data as placeholder data
            placeholderData: () => previousData,
        })

        // Keep previous data updated
        queryStore.subscribe((v) => (previousData = v.data))

        return queryStore
    }
}

You can now create reactive queries like so:

export let data

const myReactiveQuery = createReactiveQuery('myQuery', myQueryFn)

$: dataPoints = myReactiveQuery({ myData: data })
HugeLetters commented 10 months ago

@MoritzKronberger yup, did something very similar

let id: string;
let prevData: Chat;
$: chatQuery = createQuery({
  queryFn() {
    return getChat(id);
  },
  queryKey: ["chat", id],
  placeholderData: prevData,
});
// ?. because $chatQuery is undefined on initial render actually
$: if ($chatQuery?.isSuccess) {
    prevData = $charQuery.data;
}
MoritzKronberger commented 10 months ago

@HugeLetters Yes, I think your version shows the underlying "problem" of previosData being lost on re-assignment even better. But imo that is just how Svelte's reactive statements (or any JS re-assignment for that matter) work and keeping the previous state over a reactive re-assingment would just break this convention.

HugeLetters commented 10 months ago

@MoritzKronberger but react also does reassignments essentially on each rerender and it doesn't seem to cause any issues.

TkDodo commented 6 months ago

this issue looks a bit stale. please let me know how to proceed here. Otherwise I'll close it as a known limitation because the workaround seems to be "Using a store".

@lachlancollins fyi

HugeLetters commented 6 months ago

this issue looks a bit stale. please let me know how to proceed here. Otherwise I'll close it as a known limitation because the workaround seems to be "Using a store".

@lachlancollins fyi

Given that this issue might be solved completely with Svelte 5 runes I don't think this issue is severe enough to warrant a fix given there's a workaround

skelawsky commented 5 months ago

@HugeLetters I experienced the same error as described in the topic and tried to apply your solution however another problem arose: the query is fetched twice on initial render even though the query key does not change and it only happens when I add placeholderData: prevData. Did you had the same problem?

    let prevData;
    $: documentLinesQuery = createQuery({
        queryKey: ['document-lines', perPage, page, $sorting, filters],
        queryFn: async () => {
            console.log('fetch')
            let url = new URL(`document-lines`, PUBLIC_API_URL);
            appendSearchParams({ url, perPage, page, sorting: $sorting, filters });
            return await fetch(url).then((res) => res.json());
        },
        placeholderData: prevData
    });
    $: if ($documentLinesQuery?.isSuccess) {
        prevData = $documentLinesQuery.data;
    }

image