query icon indicating copy to clipboard operation
query copied to clipboard

feat(svelte-query): Svelte 5 adapter

Open zhihengGet opened this issue 11 months ago • 8 comments

i ported svelte-4 query to svelte 5

Done: (i have used below myself and they seem to work just fine)

  • createQuery
  • createMutation
  • useQueryClient

TODO:

  • createQueries (i did it but based on solid not sure if it works)
  • unit tests (pending on svelte 5 on how to test runes)
  • isFetching (svelte 4 version should be ok)
  • isMutating (svelte 4 version should be ok)
  • isRestoring (svelte 4 version should be ok)

i already published this svelte 5 draft version under svelte-query-runes package if people want to try it out ! i personally have been using createQuery and createMutation in my own project it work just fine !

to use it , you can checkout my messy sample code in examples/svelte-melt

if anyone want to take this over please feel free to do so ! i personally only need createQuery and createMutation

zhihengGet avatar Feb 27 '24 20:02 zhihengGet

@lachlancollins in case u want to want on this... fyi

there are 3 form of Reactivity approach currently

Pass Function (recommended)

let a = $state(something)
createQuery( ( )=> {  return { .... }  }   // this will be reactive for both $state and $derived and other edge cases 

Pass Attribute Function (currently, i added this for enabled+queryKey)

let a= $state()  // will work
let b=$derived()
// enabled is a fn 
createQuery({queryKey: [a] , queryFn:()=>{}, enabled:()=>{} } ) 

passing $derived to queryKey does not work i believe, if u change b, query wont update

let a= $state() 
let b=$derived()
// enabled is a fn 
createQuery({queryKey:[b] , queryFn:()=>{}, enabled:()=>{} } )  // BAD
let a= $state()  // will work
let b=$derived(a+1) // wont work if u pass b to queryKey instead of a(u have to pass ()=>b to key), other options require u to pass functions i.e enabled, 
// enabled is a fn 
createQuery({queryKey: ()=>[b] , queryFn:()=>{}, enabled:()=>{} } )  // good

pass $state to queryKey & enabled is fine too(..need to be a proxy value) inside it !

let a= $state()  // will work
createQuery({queryKey: [a] , queryFn:()=>{}, enabled:!a // if a.a is a number then enabled won't work 
} ) // queryKey can be something that has $state in it 

output of creaetQuery

we can return function or return a proxy runes , i choose the 2nd one since it has better DX , im not sure if theres some edges cases tho

zhihengGet avatar Mar 17 '24 20:03 zhihengGet

One additional thing i encountered, would be nice to fix this small formatting error in Devtools.svelte:

-<div class="tsqd-parent-container" bind:this={ref} />
+<div class="tsqd-parent-container" bind:this={ref} ></div>

Because of breaking: warn on self-closing non-void HTML tags #11114 it spams the console:

10:55:32 AM [vite-plugin-svelte] .../@[email protected]_@[email protected][email protected]/node_modules/@tanstack/svelte-query-devtools/dist/Devtools.svelte:46:0 Self-closing HTML tags for non-void elements are ambiguous — use `<div ...></div>` rather than `<div ... />`

I can open a separate pr for this, but it's not that bad, it's just annoying :smile:

max-got avatar May 02 '24 09:05 max-got

@lachlancollins @zhihengGet what is all left to get this working with latest rc? Is the current approach good?

niemyjski avatar May 08 '24 01:05 niemyjski

https://github.com/TanStack/query/discussions/7413

niemyjski avatar May 11 '24 13:05 niemyjski

@lachlancollins @zhihengGet what is all left to get this working with latest rc? Is the current approach good? cc @tannerlinsley

niemyjski avatar May 20 '24 21:05 niemyjski

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 07e64e7e73308e1d2818e66bbb811cf3cb9035f2. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
nx affected --targets=test:sherif,test:knip,test:eslint,test:lib,test:types,test:build,build --parallel=3
✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

nx-cloud[bot] avatar Jun 20 '24 07:06 nx-cloud[bot]

commit: 07e64e7

pnpm add https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@6981
pnpm add https://pkg.pr.new/@tanstack/angular-query-experimental@6981
pnpm add https://pkg.pr.new/@tanstack/eslint-plugin-query@6981
pnpm add https://pkg.pr.new/@tanstack/query-async-storage-persister@6981
pnpm add https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@6981
pnpm add https://pkg.pr.new/@tanstack/query-core@6981
pnpm add https://pkg.pr.new/@tanstack/query-devtools@6981
pnpm add https://pkg.pr.new/@tanstack/query-persist-client-core@6981
pnpm add https://pkg.pr.new/@tanstack/query-sync-storage-persister@6981
pnpm add https://pkg.pr.new/@tanstack/react-query@6981
pnpm add https://pkg.pr.new/@tanstack/react-query-devtools@6981
pnpm add https://pkg.pr.new/@tanstack/react-query-next-experimental@6981
pnpm add https://pkg.pr.new/@tanstack/react-query-persist-client@6981
pnpm add https://pkg.pr.new/@tanstack/solid-query@6981
pnpm add https://pkg.pr.new/@tanstack/solid-query-devtools@6981
pnpm add https://pkg.pr.new/@tanstack/solid-query-persist-client@6981
pnpm add https://pkg.pr.new/@tanstack/svelte-query@6981
pnpm add https://pkg.pr.new/@tanstack/svelte-query-devtools@6981
pnpm add https://pkg.pr.new/@tanstack/svelte-query-persist-client@6981
pnpm add https://pkg.pr.new/@tanstack/vue-query@6981
pnpm add https://pkg.pr.new/@tanstack/vue-query-devtools@6981

Open in Stackblitz

More templates

pkg-pr-new[bot] avatar Jul 07 '24 02:07 pkg-pr-new[bot]

Hi @zhihengGet, thanks so much for your work so far! I've moved your package from svelte-query-runes to replace the existing svelte-query folder. Would you be interested in finishing off your work on this PR?

lachlancollins avatar Jul 07 '24 02:07 lachlancollins

@lachlancollins will try to get the simple ones done sometime this week i.e devtool...

zhihengGet avatar Jul 08 '24 21:07 zhihengGet

@lachlancollins will try to get the simple ones done sometime this week i.e devtool...

Amazing! Let me know if there's anything I can do to help

lachlancollins avatar Jul 09 '24 01:07 lachlancollins

i think have moved almost everything to svelte lachlancollins early review needed!

todos if anyone want to tackle below would be great !

  1. clean up the code i.e remove unused code, consoles.info...etc
  2. fix ts
  3. svelte-persist still need migrate tests
  4. fix examples

zhihengGet avatar Jul 14 '24 05:07 zhihengGet

@lachlancollins Is there any way to allow the workflow to always run so we can install nightly builds easily. https://github.com/TanStack/query/pull/6981#issuecomment-2212293908

niemyjski avatar Jul 17 '24 02:07 niemyjski

@lachlancollins ok well i fixed them, but plz review, idk understand some of the logic, too laggy, i can't add review comments:(

zhihengGet avatar Jul 17 '24 02:07 zhihengGet

one more thing ,

i changed the output of createQuery to return final_.value instead of new Proxy( ) because console.log( ) will return {value :.... } instead of the data , too many diffs , i cant add reviews !

zhihengGet avatar Jul 17 '24 02:07 zhihengGet

@lachlancollins im not able to replicate the error locally :( i image

zhihengGet avatar Jul 17 '24 22:07 zhihengGet

@lachlancollins im not able to replicate the error locally :(

I still get it locally - maybe try running pnpm install again, or remove and re-clone your repo

 FAIL  |@tanstack/svelte-query| tests/createQueries/createQueries.test.ts > createQueries > Render and wait for success
Svelte error: state_unsafe_mutation
Updating state inside a derived is forbidden. If the value should not be reactive, declare it without `$state`

        in BaseExample.svelte

 ❯ Module.state_unsafe_mutation ../../node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/errors.js:304:17
    303|   const error = new Error(`svelte_component_invalid_this_value\nThe \`this={...}\` property of a \`<svelte:…
    304| 
    305|   error.name = 'Svelte error';
       |                ^
    306|   throw error;
    307|  } else {
 ❯ Module.set ../../node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/reactivity/sources.js:73:25
 ❯ Object.set ../../node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/proxy.js:283:26
 ❯ src/createQueries.svelte.ts:220:12
 ❯ src/createQueries.svelte.ts:219:27
 ❯ src/createQueries.svelte.ts:236:7
 ❯ Module.update_reaction ../../node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/runtime.js:296:56
 ❯ Module.update_derived ../../node_modules/.pnpm/[email protected]/node_modules/svelte/src/internal/client/reactivity/deriveds.js:86:36
 ❯ src/createQueries.svelte.ts:220:12
 ❯ src/createQueries.svelte.ts:219:27
 ❯ src/createQueries.svelte.ts:236:7

lachlancollins avatar Jul 17 '24 23:07 lachlancollins

@zhihengGet the vitest tests are all passing now, but here's quite a few type errors. Lmk if you need any help sorting these out!

lachlancollins avatar Jul 18 '24 08:07 lachlancollins

i fixed some of them ~~but seems a few packages got this -> Module '"@tanstack/svelte-query"' has no exported member 'OmitKeyof'. (ts)~~

zhihengGet avatar Jul 19 '24 00:07 zhihengGet

@lachlancollins In this test, should we expect the text to be Error: undefined? Svelte seems to render {undefined} (an undefined value in a mustache tag) to a blank string instead of the literal string "undefined".

  • https://github.com/TanStack/query/blob/5760f7002150285be37abb0f0da4af8e13071104/packages/svelte-query/tests/createMutation/createMutation.test.ts#L13
  • https://github.com/TanStack/query/blob/5760f7002150285be37abb0f0da4af8e13071104/packages/svelte-query/tests/createMutation/createMutation.test.ts#L27

thenbe avatar Jul 24 '24 10:07 thenbe

I noticed that the pattern for using createMutation is different than create query (especially around () => options).

Also, I think there might be a bug with the mutation / runes.

export function mutateDemoteTab(props: DemoteProjectTabProps) {
    const queryClient = useQueryClient();
    return createMutation<FetchClientResponse<unknown>, ProblemDetails, { name: string }>({
        mutationKey: queryKeys.id(props.id),
        mutationFn: async ({ name }) => {
            const client = useFetchClient();
            const response = await client.delete(`projects/${props.id}/promotedtabs`, {
                params: { name }
            });

            if (response.ok) {
                // Update the project to reflect the demoted tab until it's updated from the server
                const previousProject = queryClient.getQueryData<ViewProject>(queryKeys.id(props.id));
                if (previousProject) {
                    queryClient.setQueryData(queryKeys.id(props.id), {
                        ...previousProject,
                        promoted_tabs: (previousProject?.promoted_tabs ?? []).filter((tab) => tab !== name)
                    });
                }

                return response;
            }

            throw response.problem;
        },
        onSettled: () => {
            queryClient.invalidateQueries({ queryKey: queryKeys.id(props.id) });
        }
    });
}
    const demoteTab = mutateDemoteTab({
        get id() {
            return event.project_id!;
        }
    });
    
    function onDemote(title: string): void {
        demoteTab.mutate({ name: title });
    }

    $effect(() => {
        console.log('effect', demoteTab.isError, demoteTab.isSuccess);
    });

my $effect is only ever triggered once, and the demoteTab is in pending state, it never seems to be fired for success / error.

I also tried mutateAsync and the status is never reflected after it is returned.

niemyjski avatar Jul 25 '24 13:07 niemyjski

I'd encourage anyone interested to subscribe to the following discussion: https://github.com/TanStack/query/discussions/7413

lachlancollins avatar Jul 26 '24 01:07 lachlancollins