[svelte] keepPreviousData not working

[chrislcs]

Describe the bug

The keepPreviousData options does not seem to work in the svelte-query. Even when set to true the data is empty until the new query is done.

I made the query reactive using the documentation here: https://tanstack.com/query/v4/docs/svelte/reactivity

I think it is because the current syntax will rerun the createQuery method and create a new query object each time any used variable changes, instead of subscribing to change within the already created query object, but I do not know enough about the inner workings of query-core and svelte-query to know if this is indeed an issue.

Your minimal, reproducible example

https://codesandbox.io/s/nifty-cloud-pxwjho?file=/App.svelte

Steps to reproduce

1. Create a reactive query with a queryKey with a variable and keepPreviousData set to true
2. Render the results to the DOM
3. Update the variable
4. All results are removed immediately removed from the DOM
5. And rendered again when the new query result is available

Expected behavior

As a user, I expected the previous data be visible in the DOM during fetching of the updated data

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

• OS: Windows
• Browsers: Chrome 109.0, Firefox 109.0

TanStack Query version

4.22.3

TypeScript version

4.9.4

Additional context

No response

[lachlancollins]

Interesting - I think solid-query gets around this by defining the inline options object as reactive, while in svelte you need to define the whole line as reactive.

I wonder if defining the options before calling createQuery fixes this issue? So it would look like $: options = {...}, then createQuery(options). I'll try this in the morning :)

[chrislcs]

I tried that and it didn't work for me. I had a deeper look into this and, among other things, checked how tanstack/table handles it and they seem to use a writable store for the entire options object. I think this would also be a good idea for svelte-query. It would look something like this:

const options = writable({
  queryKey: ['test', [1]],
  queryFn: async ({ queryKey }) => {
    await sleep(100)
    return queryKey[1]
  },
  keepPreviousData: true,
})
const query = createQuery(options)

Within svelte files you can then simply update the options like so:

$options.queryKey[1] = [1, 2]

And outside of svelte files:

options.update((o) => ({ ...o, queryKey: ['test', [1, 2]] }))

If you do not need the reactivity you can still use a regular options object.

I made a PoC with a test that was failing before and working with these changes. I do not know query-core well though, so I'm not sure all changes are completely correct. You can check this here: main...chrislcs:query:main

[lachlancollins]

This looks great :) The only bit that I'm a bit concerned about is forcing the types on parseQueryArgs, as this would also need to be done for the other functions that use createBaseQuery and have more complex types. However in v5, there will be no passQueryArgs, so maybe this could be a v5 feature - do you want to submit a PR into the v5 branch?

EDIT: I've also just checked this never worked with the old package either.

[chrislcs]

Yeah forcing the types isn't pretty, was just meant as a PoC. With the current implementation of parseQueryArgs:

  if (!isQueryKey(arg1)) {
    return arg1 as TOptions
  }

  if (typeof arg2 === 'function') {
    return { ...arg3, queryKey: arg1, queryFn: arg2 } as TOptions
  }

  return { ...arg2, queryKey: arg1 } as TOptions

it shouldn't cause any problem since called like createQuery(writableOptions) will return on the first condition, createQuery('test', writableOptions) will pass the first and second condition correctly (since a writable is an object, not a function) and createQuery('test', () => 1, writableOptions) will correctly return on the second condition. It'd be fairly easy to copy over this utility function to svelte-query and make a typesafe version of it.

EDIT: Scratch that. Now that I thought about it more, it wouldn't work if the writeableOptions are not the first argument. We'd need to update the writable with the other arguments if it isn't passed as the first argument. Still not too difficult, but more substantial. Worth it to do that or just go for v5?

I can make a PR tomorrow 👍

[lachlancollins]

EDIT: Scratch that. Now that I thought about it more, it wouldn't work if the writeableOptions are not the first argument. We'd need to update the writable with the other arguments if it isn't passed as the first argument. Still not too difficult, but more substantial. Worth it to do that or just go for v5?

I think probably just go for v5, thanks for figuring this out! Also I doubt this would fix the problem since createQuery still gets re-run, but keepPreviousData will be merged into placeholderData in v5: #4715

[chrislcs]

Sorry for the sudden lack of activity from my side, I caught the flu and was unable to work on this for a while. I hope to make the PR soon.

[tmarnet]

How would you make the query reactive to external variables (as opposed to user events such as the on:input from the doc) - like the URL's search params (e.g. $page.url.searchParams in SvelteKit) - while keeping the behaviour of keepPreviousData ?

By following the new reactivity documentation the only thing that works for me is the following, but that doesn't feel very intuitive:

  const ids = $page.url.searchParams.getAll(name)

  const queryOptions = writable({
    queryKey: ['reactive', ...ids],
    queryFn() {
      return fetchWithIds(ids)
    },
    placeholderData: keepPreviousData,
  })

  const query = createQuery(queryOptions)

  $: {
    const nextIds = $page.url.searchParams.getAll(name)

    $queryOptions = {
      queryKey: ['reactive', ...nextIds],
      queryFn() {
        return fetchWithIds(nextIds)
      },
      placeholderData: keepPreviousData,
    }
  }

Am I missing something ?

Thanks !

[chrislcs]

You should use the provided queryKey in the query function. This way you do not have to update the query function.

Something like this (quickly wrote this, not tested):

  const ids = $page.url.searchParams.getAll(name)

  const queryOptions = writable({
    queryKey: ['reactive', ids],
    queryFn: ({ queryKey }) => fetchWithIds(queryKey[1]),
    placeholderData: keepPreviousData,
  })

  const query = createQuery(queryOptions)

  $: {
    const nextIds = $page.url.searchParams.getAll(name).map(Number)
    $queryOptions.queryKey[1] = nextIds
  }

[tmarnet]

While this is indeed a bit shorter, it still doesn't feel as right as leveraging reactive statements $: like before - but I guess we don't really have a choice when it comes to using keepPreviousData.

Nonetheless, this now raises the question of typings since we don't benefit from autocompletion in writable() and the queryKey (amongst other things) raises an error:

Binding element 'queryKey' implicitly has an 'any' type. ts(7031)

Any idea of to make it work seamlessly with TS ?