ConsoleTVs / sswr

🔥 Svelte stale while revalidate (SWR) data fetching strategy
MIT License
234 stars 11 forks source link

Dependent fetching just doesn't work? #24

Open tmladek opened 2 years ago

tmladek commented 2 years ago

If I clone the default Svelte template via npx degit sveltejs/template my-svelte-project, then yarn add sswr, and copy&paste the dependent fetching example from the README into "App.svelte"...

<script>
  import { useSWR } from "sswr";

  const { data: post } = useSWR("https://jsonplaceholder.typicode.com/posts/1");
  // We need to pass a function as the key. Function will throw an error when post is undefined
  // but we catch that and wait till it re-validates into a valid key to populate the user variable.
  $: ({ data: user } = useSWR(
    () => `https://jsonplaceholder.typicode.com/users/${$post.userId}`
  ));
</script>

{#if $post}
  <div>{$post.title}</div>
{/if}
{#if $user}
  <div>{$user.name}</div>
{/if}

Only the $post store ever gets populated - and what's more, the https://jsonplaceholder.typicode.com/users/${$post.userId} URL never seems to get fetched.

image

I was, actually tracking down a bug I thought was on my side - I'm changing the key based on a reactive variable (which in turn is set based on a useSWR fetch) and not a SWR-returned store directly (as in the example). So, I thought that the fact the second fetch never happens is a consequence of the way the reactivity is set up (since useSWR() uses onMounted(), I presume a useSWR() call which happens later (i.e. when my reactive variable is updated) doesn't get hooked properly) - but now I'm not so sure. This may be a separate issue entirely, in which case I will delete this portion of the issue.

Thanks in advance!

ConsoleTVs commented 2 years ago

A lot of people seem to be getting this as well. Will investigate.

mgarf commented 2 years ago

@ConsoleTVs was this ever fixed? i feel like i'm seeing the error and before i dig deeper wanted to check if maybe its still outstanding issue

ConsoleTVs commented 2 years ago

It might be. I am in the process of updating the underlying library of both, sswr and vswr. Althought this seems to be related to svelte itself, this library is around 70 LOC specific svelte code. It should not be hard to spot either if you want to help!

Also, could somebody post a replication code on codesandox?

ekzhang commented 2 years ago

Unfortunately I don't have a replication at the moment, but the error might have to do with the fact that onMount is called in the constructor? If useSWR is reactively called behind a $: in the dependent fetching scenario, then it would be rerun later in the component lifecycle after the mount has already happened.

mgarf commented 2 years ago

i might have some time today to dig into this. though im in the early stages of learning svelte, most of my framework knowledge is react, 🤷 doesnt hurt to try

mgarf commented 2 years ago

dropped in

    beforeUpdate(() => {
      console.log('key', key, this.resolveKey(key), options);
      this.use<D, E>(key, onData, onError, {
        loadInitialCache: true,
        ...options,
      })
    });

and moved onData and on Error to be outside of on mount. The good news is that the fetching is now fixed. the problem is we are creating waaay to many subscriptions... so just need to find the underlying refetch for this.use that i can use on the existing subscription

mgarf commented 2 years ago

ok removed my changes and replace onMount with beforeUpdate and added an if condition to check if the subscription has been created. @ekzhang was right the onMount is happening too early

    beforeUpdate(() => {
      // Handlers that will be executed when data changes.

      // Subscribe and use the SWR fetch using the given key.

      if (!unsubscribe)
        unsubscribe = this.use<D, E>(key, onData, onError, {
          loadInitialCache: true,
          ...options,
        }).unsubscribe
    })

I'll create a PR for the above change but I'm not a huge fan of it since it seems hacky. Going to also do some extra testing as well to make sure it doesn't have some weird side effects

eytanProxi commented 2 years ago

Still doesn't work ... 😢

ConsoleTVs commented 2 years ago

@eytanProxi can you manually test this branch? If it works I can merge it: https://github.com/ConsoleTVs/sswr/pull/33

eytanProxi commented 2 years ago

@ConsoleTVs : I tried the fix but the bug is still here. Should we move to TurboQuery ?

ConsoleTVs commented 2 years ago

I have no idea how to fix that. You can give turbo query a shot, it is basically using Promises. Should be easy to create a small adapter. I am pushing turbo query beyond with vue and specially solidjs as a replacement for this...

smblee commented 2 years ago

@ConsoleTVs are you referring to https://github.com/StudioLambda/TurboQuery?