query icon indicating copy to clipboard operation
query copied to clipboard

feat(svelte-query): Svelte Query Adapter for TanStack Query

Open drejohnson opened this issue 1 year ago • 15 comments

This pull request adds Svelte adapter for TanStack Query.

Primitives included in the package are:

  • useQueryClient
  • useQuery
  • userQueries
  • useMutation
  • useInfiniteQuery
  • useHydrate
  • useIsMutating
  • useIsFetching

A few examples have been included as well as a few initial tests for useQuery

TODO

  • [ ] Add comprehensive test
  • [ ] Add docs and readme
  • [ ] Add more examples

Acknowledgment

Thanks to:

  • @amen-souissi and all the other contributors for the work they put into svelte-query

drejohnson avatar Oct 19 '22 16:10 drejohnson

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit bdfeac4727a71cdc6082c710212483376f074be7:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

codesandbox-ci[bot] avatar Oct 19 '22 16:10 codesandbox-ci[bot]

please add the basic example to ci.json so that we get preview builds:

https://github.com/TanStack/query/blob/4ae99561ca3383d6de3f4aad656a49ba4a17b57a/.codesandbox/ci.json#L3

TkDodo avatar Oct 22 '22 14:10 TkDodo

Codecov Report

Base: 96.36% // Head: 90.72% // Decreases project coverage by -5.63% :warning:

Coverage data is based on head (bdfeac4) compared to base (eab6e2c). Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4344      +/-   ##
==========================================
- Coverage   96.36%   90.72%   -5.64%     
==========================================
  Files          45       97      +52     
  Lines        2281     3719    +1438     
  Branches      640      911     +271     
==========================================
+ Hits         2198     3374    +1176     
- Misses         80      326     +246     
- Partials        3       19      +16     
Impacted Files Coverage Δ
src/core/infiniteQueryBehavior.ts
src/core/utils.ts
src/core/queriesObserver.ts
src/core/query.ts
src/react/setLogger.ts
src/core/queryObserver.ts
src/core/retryer.ts
src/devtools/tests/utils.tsx
src/core/mutation.ts
src/core/queryClient.ts
... and 132 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Oct 23 '22 17:10 codecov-commenter

This looks great, thanks for making it happen, I'm very excited to use this ❤️

I'd like to suggest an API change to make it feel more Svelte-like. The prefix of use is really a React thing, implying that you're going to receive a hook. This isn't a concept in Svelte since rendering doesn't happen by calling a function to emit DOM nodes (e.g React) and it makes the API feel a bit unconventional to Svelte users.

In Svelte, when working with data, most of the operations are either setting/getting, either as state or subscribing/updating a store, donated by the $ prefix.

For example, the equivalent of useContext in Svelte is setContext and getContext and it does exactly as it says on the tin, and when working with a readable such as let store = readable(...) it will be accessed with $store. Whereas useQuery() could simply be query() or queryStore(), or useIsMutating could be $isMutating.

One of my favourite examples of this is Urql. The React API uses useQuery and useMutation and Svelte API uses queryStore and mutationStore.

What do you think?

ivanvanderbyl avatar Oct 27 '22 23:10 ivanvanderbyl

I'm excited for this and thankful. @drejohnson, make sure to change Solid-> Svelte in the package.json

mattlehrer avatar Nov 01 '22 13:11 mattlehrer

@drejohnson anything I can help with to move this along?

ivanvanderbyl avatar Nov 04 '22 23:11 ivanvanderbyl

Sorry guys, I've been on vacation. I'll update the PR sometime this week to get this moving along

drejohnson avatar Nov 07 '22 13:11 drejohnson

This is looking great! Thanks for adding the examples. I'm not familiar with Svelte as much but seems like their reactivity model is similar to Solid. Let me know if you need any help getting the tests we wrote for solid ported to the svelte-query package :D

Thanks, I may have to take you up on your offer

drejohnson avatar Nov 07 '22 13:11 drejohnson

@drejohnson anything I can help with to move this along?

Thanks! Are you familair with rollup? I can't figure out how to package .svelte components. That and writing tests :D

drejohnson avatar Nov 07 '22 13:11 drejohnson

This looks great, thanks for making it happen, I'm very excited to use this ❤️

I'd like to suggest an API change to make it feel more Svelte-like. The prefix of use is really a React thing, implying that you're going to receive a hook. This isn't a concept in Svelte since rendering doesn't happen by calling a function to emit DOM nodes (e.g React) and it makes the API feel a bit unconventional to Svelte users.

In Svelte, when working with data, most of the operations are either setting/getting, either as state or subscribing/updating a store, donated by the $ prefix.

For example, the equivalent of useContext in Svelte is setContext and getContext and it does exactly as it says on the tin, and when working with a readable such as let store = readable(...) it will be accessed with $store. Whereas useQuery() could simply be query() or queryStore(), or useIsMutating could be $isMutating.

One of my favourite examples of this is Urql. The React API uses useQuery and useMutation and Svelte API uses queryStore and mutationStore.

What do you think?

Yeah, I agree, I initially thought about doing this.

drejohnson avatar Nov 07 '22 13:11 drejohnson

Good changes,

I have some feedbacks:

  1. getQueryClientContext and setQueryClientContext should not be exported in index.ts
  2. store.ts is not needed anymore
  3. useQueryClient should not be responsible of mount/unmount. It should be done directly in setQueryClient. useQueryClient is just a shortcut to getQueryClientContext
  4. we should implement methods: setOptions, updateOptions, setEnabled on BaseQuery and on all children classes

What do you think?

SomaticIT avatar Nov 11 '22 21:11 SomaticIT

Any update on this?

emarj avatar Dec 04 '22 12:12 emarj

Hi all, I'm wondering if it would be possible to merge this PR without releasing the package on NPM just yet? It seems like the core functionality is all here (@drejohnson has done an amazing job!), but the final work might benefit from allowing others to contribute their own PRs in this repository.

I think it would be awesome to have this released close to the SvelteKit 1.0 release (ETA around 5 days), and I'm sure a lot of people trying it out from a React background would be happy to see TanStack Query available!

Based on this thread, it seems the final things that are still to-do are fixing some memory leaks, considering renaming exported functions (e.g. useQuery vs createQuery vs queryStore), adding some missing methods, and more tests.

lachlancollins avatar Dec 09 '22 23:12 lachlancollins

@lachlancollins we could also make PRs to this PR ?

we could merge the PR and not release it, but any further PR that gets merged to main that has a fix or feat commit would release it automatically. This would severely hinder us in development. Also, the failing tests would block any further merges.

So if you want to get this merged, please contribute to this PR.

TkDodo avatar Dec 10 '22 07:12 TkDodo

Hello! Is it possible to have a list of the specific tasks that are needed to complete for this PR to be merged? I see there are 3 todo items in the description but I would like more information so I can contribute if possible. Thank you!

romelperez avatar Dec 17 '22 03:12 romelperez

This PR is stalled a bit, but we'd really like to get it over the finish line. I honestly don't know what is missing to get that done, mainly, the question would be if the author @drejohnson is still around to finish it, or if someone else would pick it up?

What we would need is someone to own the svelte-query adapter and who's willing to stick around to maintain it after the first release. We're also working on v5 already and I wouldn't want to merge this to main without someone being able to help with v5 work.

TkDodo avatar Dec 30 '22 21:12 TkDodo

@lachlancollins we could also make PRs to this PR ?

we could merge the PR and not release it, but any further PR that gets merged to main that has a fix or feat commit would release it automatically. This would severely hinder us in development. Also, the failing tests would block any further merges.

So if you want to get this merged, please contribute to this PR.

Hi @TkDodo, thanks for your reply - when I want to merge my changes into this current PR, do I need to make a new PR into TanStack/query or into drejohnson/query? Thanks!

lachlancollins avatar Jan 05 '23 02:01 lachlancollins

@lachlancollins depends on what is easier for you. If you think it's starting from scratch, I'll close this PR and you can copy the commits over to a new PR. If you want to make a PR to this PR that's also fine for me.

TkDodo avatar Jan 05 '23 08:01 TkDodo

Im willing to contribute in any way possible as well. Let me know if you want to divide and conquer this @lachlancollins

smblee avatar Jan 05 '23 17:01 smblee

Hi @TkDoko, I've just made a new PR at #4768. I think it might be easier to use a new PR so that we don't have to make multiple PRs here for each change.

@smblee I would definitely appreciate some help! Please have a look at the to-do list as a starting point :)

lachlancollins avatar Jan 05 '23 20:01 lachlancollins

superseded by #4768

TkDodo avatar Jan 07 '23 11:01 TkDodo