SvelteStack / svelte-query

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

Provide examples for useQuery with reactive values #88

Open pzcfg opened 2 years ago

pzcfg commented 2 years ago

After lots of debugging headaches https://github.com/SvelteStack/svelte-query/discussions/87 we discovered that we weren't using useQuery correctly when passing in reactive values.

We searched all sorts of different keywords online, and the only reference I could find to reactive values in queries was this one issue https://github.com/SvelteStack/svelte-query/issues/43

Could you please add examples for the docs using dependent queries with reactive values?

pzcfg commented 2 years ago

https://svelte.dev/repl/9fe7b9ef36f846ee8a807ad2b0d32c43?version=3.48.0

I'm pretty sure you're using this incorrectly and your fetch is only happening because refetchOnWindowFocus defaults to true.

You change your queryParams.page, but const queryResult = useQuery() is not reactive so it only runs when the component is initialized and isn't going to be re-run when parameters change. It seems the only reason yours is working is because fetchPassengers accesses queryParams.page directly and doesn't parse it from the query parameters given to useQuery().

dysfunc commented 2 years ago

You're right. My example was invalid, lol. I literally forgot to add the setOptions reactive binding 🤦🏻‍♂️ I've updated that REPL, and removed my previous comment.

dysfunc commented 2 years ago

Curious, based on the updated REPL, what else are you looking for? Is it just documentation or a better way to set things up?

pzcfg commented 2 years ago

Part of it is documentation/examples. We couldn't find any of either for updating queries with new values. Attempting to follow the react-query paradigms lead us to trying to write reactive queries like $: query = useMyQuery(changingValue), which sort of worked but ended up causing a lot of other issues. We only stumbled up on setOptions / updateOptions after some lucky search terms lead us to #43.

Additionally we were having a lot of issues that seem to stem from #89/#91, but it doesn't seem like anyone is looking at issues/PRs?

In general though, the svelte-query way of doing things involves us duplicating a lot of configuration at both the useQuery and setOptions/updateOptions case. We created a bunch of query functions that hide away the implementation details of the query building, e.g.

const queryKeys = {
  floor: (site, building, floor) => ['floor', {site, building, floor}]
}

const fetchFloor = ([, {site, building, floor}]) => fetch(`/api/${site}/${building}/${floor}`)

export const useRoomQuery = (site, building, floor, room) => useQuery(
  queryKeys.floor(site, building, floor),
  {
    queryFn: fetchFloor
    select: (data) => {
      return data.filter(item => item.room_number === room)
    }
  }
)

In an ideal world, we'd just have something like $: query = useRoomQuery(site, building, floor, room) and it'd just fetch/select/etc. as needed when the variables change. This even sort of works, but it sounds like it's not supposed to be used this way? https://github.com/SvelteStack/svelte-query/issues/43#issuecomment-894770920

Instead, we need to duplicate the query key logic and select function in .updateOptions(), like:

const query = useRoomQuery(site, building, floor, room)

$: query.updateOptions({

  // now the query key implementation details need to be pulled into the application/component layer
  queryKey: queryKeys.floor(site, building, floor),

  // now the select function needs to be defined both in useQuery and any callsite it's used at
  select: (data) => {
    return data.filter(item => item.room_number === room)
  }

})
ericpgreen2 commented 2 years ago

Ditto, I'd love to see some best practices documentation for custom hooks & reactive values!

I'd like my component code to look similar to react-query component code, as in @pzcfg's example above: $: query = useRoomQuery(site, building, floor, room)

What's the proper svelte-query pattern for these custom hooks? Can we idempotently call useQuery? Or should we call useQuery the first time, then setOptions/updateOptions thereafter?

MoritzKronberger commented 2 years ago

I totally agree with @pzcfg, the current solution of using updateOptions produces tons of code duplication and is not documented anywhere (apart from a hint in the dependent queries docs). But since using $: query = useMyQuery(changingValue) means that the entire query is re-created with every change of the reactive value and previousData is therefore discarded, etc., I think it's still the only way to get reactive query updates fully working.

Some (more or less helpful) suggestions to reduce the code duplication:

I found it unnecessary to repeat select inside updateOptions. Looking at the implementation of updateOptions it also seems like previous query options should be retained.

Although it can definitely be improved by a lot, I'd like to throw my adaptation of the generic useQuery wrapper out there. It simplifies creating custom query hooks, that manage the query key, as well as extend the object returned by useQuery with an update property, that allows reactively updating the query with less boilerplate.

type GenericUseQueryOptions<TQueryData, TError, TData, TKey, TQueryInput> =
  Omit<
    UseQueryOptions<TQueryData, TError, TData, [TKey, TQueryInput]>,
    'queryKey' | 'queryFn'
  >

export const createCustomHook = <
  TKey extends string,
  TQueryData,
  TError,
  TQueryInput = void,
  TData = TQueryData
>(
  key: TKey,
  query: (input: TQueryInput) => Promise<TQueryData>
) => ({
  key,
  useQuery: (
    input: TQueryInput,
    options?: GenericUseQueryOptions<
      TQueryData,
      TError,
      TData,
      TKey,
      TQueryInput
    >
  ) => {
    const queryStore = useQuery({
      queryKey: [key, input],
      queryFn: (context) => query(context.queryKey[1]),
      ...options,
    })

    return {
      ...queryStore,
      // Add the reactive update helper
      update: (input: TQueryInput) =>
        queryStore.updateOptions({ queryKey: [key, input] }),
    }
  },
})
const myHook = createCustomHook('myQuery', (input: ...) =>
  myQuery(input)
)

const query = myHook.useQuery(reactiveInput, { select: (data) => ... })

$: query.update(reactiveInput)
pzcfg commented 2 years ago

I found it unnecessary to repeat select inside updateOptions.

You need to repeat the select function if you're changing what it's selecting. In my example above the room value being compared in item.room_number === room would not be updated unless you pass in a new select function.

I'd like to throw my adaptation of the generic useQuery wrapper out there.

Yeah we ended up building something similar to this, although were still having some issues with it. In our case we still wanted to update more than just the queryKey and so I think we still ended up having to write or at least pass in new select statements in multiple places.

niemyjski commented 1 year ago

+1, this is a pain to the point I find I'd contemplating just use fetch. It is not idiomatic at all.