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

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

Closed
HugeLetters opened this issue Aug 26, 2023 · 13 comments
Closed

Comments

@HugeLetters
Copy link

HugeLetters commented Aug 26, 2023

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

2023-08-26.13-42-19.mp4

Platform

  • Windows
  • Google Chrome

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
Copy link
Collaborator

TkDodo commented Aug 28, 2023

@lachlancollins FYI

@james-kaguru
Copy link

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
Copy link
Author

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
Copy link

@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
Copy link
Contributor

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.

@HugeLetters
Copy link
Author

HugeLetters commented Oct 19, 2023

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
Copy link

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
Copy link
Author

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

@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
Copy link
Author

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

@TkDodo
Copy link
Collaborator

TkDodo commented Feb 17, 2024

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
Copy link
Author

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

@HugeLetters HugeLetters closed this as not planned Won't fix, can't repro, duplicate, stale Feb 17, 2024
@skelawsky
Copy link

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants