ConsoleTVs / sswr

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

add `stop`, `isLoading` and `isValid` #19

Closed frederikhors closed 2 years ago

frederikhors commented 3 years ago

~Fixes 4.~

~@kevmodrome what do you think?~

I reverted the rename as requested by @ConsoleTVs.

This PR now only adds stop, isLoading and isValid.

ConsoleTVs commented 3 years ago

It needs some more thinking, if somebody creates a custom instance, I want it to have exactly the same method names available to him. Could we rename the method name from useSvelte to useSWR as https://github.com/ConsoleTVs/vswr/blob/master/src/index.ts#L8

I also just noticed I did not expose all methods by default, check: https://github.com/ConsoleTVs/vswr/blob/master/src/index.ts

If you could address this, I would be really happy!!

I want to have both libraries in the same level of quality!

frederikhors commented 3 years ago

I added stop, isLoading and isValid.

I cannot understand stop() from vswr. Can you better explain what it does?

ConsoleTVs commented 3 years ago

It just unsubscribes from the cache, avoids updating the value if a value in the cache is changed

frederikhors commented 3 years ago

Sorry I meant loading(). stop() is done.

ConsoleTVs commented 3 years ago

isLoading will be true whenever a request has started to fetch the data but it's not yet completed (the promise is not yet resolved). Basically there's no data nor error yet. This is where you would show some placeholder data like those loading animation: https://tailwindcss.com/docs/animation#pulse

ConsoleTVs commented 3 years ago

I have updated the branch, resolve the conflicts and we're good to with those additions. Regarding the name change, I'll like to keep the useSWR base name. If somebody wants to use another name, just create a custom instance and re-export the methods with a custom name. I understand it's not the best way to go on svelte, but I just want to maintain some level of stability between sswr, vswr and swrev.

frederikhors commented 3 years ago

isLoading will be true whenever a request has started to fetch the data but it's not yet completed (the promise is not yet resolved). Basically there's no data nor error yet. This is where you would show some placeholder data like those loading animation: https://tailwindcss.com/docs/animation#pulse

isLoading is clear to me.

I am asking for info about loading I see in vswr.

If you think we should add it here too.

frederikhors commented 3 years ago

I'll like to keep the useSWR base name

So I can revert the first commit? I'll leave it as it was, right?

If you can, rename it to useSWR instead of useSvelte (to maintain naming conventions). But yes, I'll like to keep the use* thing for clarity among library-specific implementations of swrev.

frederikhors commented 3 years ago

I reverted the rename.

This PR now only adds stop, isLoading and isValid.

@ConsoleTVs you edited my comment instead of a new answer! 😄

ConsoleTVs commented 3 years ago

I reverted the rename.

This PR now only adds stop, isLoading and isValid.

@ConsoleTVs you edited my comment instead of a new answer! 😄

Oh my god, I am so sorry!!! I used the new github ios app and I kinda screwed up!

Looking good, gonna merge later, thanks a lot!!!!

frederikhors commented 3 years ago

gonna merge later

Merge it now!

I want to break the world with this project! 😄😄😄

benbender commented 3 years ago

Just as a note: most of those state-booleans are mutually exclusive and should be internally combined at some point to reflect that. F.e. if a query is loading, it can't be valid or invalid. If that's not reflected, there will be edge-cases at some point. I would suggest to basically use a finite state-machine internally to determine exactly which state is actually true and which state-transitions can happen at any given point in time according to that state.

frederikhors commented 3 years ago

Thanks for the thought, @benbender.

I, too, had initially thoughts for a finite state machine.

But for now it seems exaggerated. Maybe in the near future, when will we have more states to manage?

frederikhors commented 3 years ago

If a query isLoading it can be valid or not. I think in this case isStale is a better name.

benbender commented 3 years ago

The request can't be valid as the result of the request isn't determined yet while it's loading... In state-machine terms it may be in state "loading" and could transition to (f.e.) "success", "canceled" or "error". It can only have one of those states at a given point in time.

Things like "fresh" or "stale" could be states of an associated cache-object (which might be related in a common stategraph) but aren't states of the request itself (even if the result of the request could influence the state of the associated cache-object or vice versa).

Statemachines are great for those problems as they provide you with a terminology and system to think about (and maybe visualize) those problems.

See: https://xstate-catalogue.com/machines/simple-data-fetch

And as always: naming is hard ;)