feat: Svelte 5 API proposal #8852
Conversation
docs(svelte-5-adapter): force runes mode for all examples (TanStack#7800) * docs(examples): force runes mode for all examples * Fix simple example mounting * Fix type feat(svelte-5-adapter): require options to be passed as function (TanStack#7804) * fix(svelte-5-adapter): Require options to be passed as function * More fixes * More fixes * Update examples fix(svelte-5-adapter): function for `createMutation` options (TanStack#7805) * fix(svelte-5-adapter): require function for createMutation options * Fix type errors refactor(svelte-5-adapter): simplify createBaseQuery (TanStack#7808) * refactor: simplify createBaseQuery * Don't take snapshot of queryKey chore(svelte-5-adapter): tidy-up functions (TanStack#7809) * chore(svelte-5-adapter): tidy-up functions * Fix types * Update svelte Fix import extensions chore: ignore state_snapshot_uncloneable in tests Fix merge
|
View your CI Pipeline Execution ↗ for commit 258d992.
☁️ Nx Cloud last updated this comment at |
…-the-longest-name-on-github/tanstack-query into svelte-5-adapter-derived-by
|
Isn't it a bad thing to have so many $effects? Also how does this work for export scenarios like this where all of my queries are exported https://github.com/exceptionless/Exceptionless/blob/main/src/Exceptionless.Web/ClientApp/src/lib/features/projects/api.svelte.ts currently feels weird and I've been trying to rework params... |
Can you expand upon what you mean? There aren't many effects here, right? Effects in and of themselves aren't bad things, they just become bad things if misused. In this case, there's really no way to synchronize state with an external system (
Basically, you'd just wrap those query exports in const { data, isLoading } = $derived.by(getOrganizationProjectsQuery(...)); |
const data = createQuery(()=>{ ... }) //passing function and directly mutate data for optimistic updates, not sure if it is the best practice tho
const { data, isLoading, ...etc } = $derived.by( // if u want to spread it... then you need $derived.by, but i prefer not doing it
data
);const { data, isLoading, ...etc } = $derived.by( // you can't directly mutate data I believe ? in recent version, we can assign stuff to $derived for optimistic updates but mutate $state feels better? subjective
createQuery({ queryKey, queryFn })
); |
As far as optimistic updates go:
|
|
Doesn't really matter to me but feels like it's way less idiomatic and I don't like seeing framework code. |
|
I'm curious -- what idioms are you referring to? The alternative API is basically what we had before: const query = createQuery(() => ({ queryKey, queryFn }))Downsides being:
As far as idioms go, you basically need a thunk on the way in (to preserve argument reactivity) or on the way out (to preserve return value reactivity) -- but not both. Svelte is designed to handle thunk returns gracefully with Either way, the optimistic updates thing above applies -- this shouldn't be implemented with |
|
I find the example from proposal very elegant 👍 Avoiding the need to use functions for the arguments is amazing, and since queries do depend on outside variables (like the query key) it does make perfect sense that you'd need to wrap it in a |
Having $derived.by at every callsite doesn't feel like just writing javascript (idomatic). And I agree I don't like passing functions either (which seems worse) and not intuitive on what should be a function or not. I have not been destructuring at all here because I haven't seen anyone else do it for tanstack nor the docs (unless they've been updated). I'm always concerned about broken references e.g., isSuccess (computed) being updated properly when destructured? I agree with this as well:
On final note, I'm fine with what ever design but for the sake of porting docs / and samples would be great to have the least amount of changes as possible as svelte doesn't get much love from tanstack docs/community updating those (And I get I could contribute there as well, time is very very limited). Thanks again! |
I just want to point out that "every callsite" is not technically correct, though I expect what you mean is probably true. This would absolutely work: const query = createQuery({ queryKey, queryFn })...you just can't destructure it, and More practically, though, you definitely don't need it when creating your own queries: function createUserQuery({ id }) {
return createQuery({
queryKey: ['user', id],
queryFn: ({ queryKey }) => fetch(`/api/users/${queryKey[1]}`).then(r => r.json)
})
}...you would just need it where you actually use the query: const { data, isLoading } = $derived.by(
createUserQuery({ id })
)
To answer the surface-level question, so long as you're using
This has been the beast I've had to struggle with, especially the intuitiveness part. Maybe for some insanely-bleeding-edge, maximum-possible-performance-needed scenarios it makes sense to fully take advantage of fine-grained reactivity by passing individual getters or functions across the hook boundary, but Tanstack Query is not that use case, especially since all of its state is already externally hosted in
Yeah definitely, this won't go live without updated docs and examples. I'm actually working on porting all of the React tests as well, as the current coverage is pretty abysmal. |
There was a problem hiding this comment.
Thanks so much for your work @elliott-with-the-longest-name-on-github ! After a fair bit of thinking, I agree this syntax is the more svelte v5-appropriate choice. @arnolicious 's comment sums it up well:
Avoiding the need to use functions for the arguments is amazing, and since queries do depend on outside variables (like the query key) it does make perfect sense that you'd need to wrap it in a
derived(derived.by), like any other derived value.
Giving a big 👍 to keep working on this!
Also I think it would be more appropriate for the PR to target the TanStack:svelte-5-adapter branch, so we're comparing the two runes implementations.
|
I'll re-target the branch -- I had it pointing at |
|
How will using variables in queryKey work without createQuery accepting a callback? let search = $state('')
const query = createQuery(() => ({
queryKey: ['products', search],
queryFn: async () => {
const data = await fetch(`https://dummyjson.com/products/search?q=${search}`).then((r) => r.json())
return data.products
})But in this version I couldn't make it work |
Since createQuery now returns a thunk ( const query = $derived.by(createQuery({
queryKey: ['products', search],
queryFn: async () => {
const data = await fetch(`https://dummyjson.com/products/search?q=${search}`).then((r) => r.json())
return data.products
})); |
|
The problem is not how to use the result of createQuery, but how to make it reactive to the properties of the object it receives as a parameter |
|
@Thiagolino8 oh I see... Yeah, that's a problem. You can get around it by passing your own thunk to |
This would recreate the query on every state change |
|
In this case, I feel like the original solution of passing the query options as a thunk might be more ideal. Another alternative I can think of would be to support passing a thunk to Given that the solid adapter already sets a precedent for passing a thunk to |
|
I agree. The original solution is a much better DX than the derived one. It
also wouldn’t be an unprecedented API from a Tanstack Query perspective as
the Angular variant also requires a thunk.
…On Mon, Mar 31, 2025 at 7:28 PM David ***@***.***> wrote:
In this case, I feel like the original solution of passing the query
options as a thunk might be more ideal.
Another alternative I can think of would be to support passing a thunk to
queryKey and/or values as thunks within the queryKey array. But then
using the values in queryFn feels sort of disjointed.
Given that the solid adapter already sets a precedent for passing a thunk
to createQuery, I feel that might be the superior solution after all.
There is also precedent among Svelte maintainers
<sveltejs/svelte#12800 (comment)>
that this is the way to go for maintaining reactivity.
—
Reply to this email directly, view it on GitHub
<#8852 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AY6KRUCZRJ7FNNC62PQ44S32XHFRFAVCNFSM6AAAAABZWVCG4SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRXGY2DEOBWGQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
[image: hmnd]*hmnd* left a comment (TanStack/query#8852)
<#8852 (comment)>
In this case, I feel like the original solution of passing the query
options as a thunk might be more ideal.
Another alternative I can think of would be to support passing a thunk to
queryKey and/or values as thunks within the queryKey array. But then
using the values in queryFn feels sort of disjointed.
Given that the solid adapter already sets a precedent for passing a thunk
to createQuery, I feel that might be the superior solution after all.
There is also precedent among Svelte maintainers
<sveltejs/svelte#12800 (comment)>
that this is the way to go for maintaining reactivity.
—
Reply to this email directly, view it on GitHub
<#8852 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AY6KRUCZRJ7FNNC62PQ44S32XHFRFAVCNFSM6AAAAABZWVCG4SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRXGY2DEOBWGQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Ahh rats, I think you're right. I had been basing my tests off of the behavior in the it(
'keeps up-to-date with query key changes',
withEffectRoot(async () => {
let search = $state('')
const states: Array<CreateQueryResult<string>> = []
const query = createQuery(
() => ({
queryKey: ['products', search],
queryFn: async () => Promise.resolve(search),
}),
queryClient,
)
$effect(() => {
states.push({ ...query })
})
await vi.waitFor(() => expect(query.data).toBe(''))
search = 'phone'
await vi.waitFor(() => expect(query.data).toBe('phone'))
console.log(
states.map((s) => ({
data: s.data,
status: s.status,
fetchStatus: s.fetchStatus,
})),
)
expect(states.length).toBe(4)
expect(states[0]).toMatchObject({
status: 'pending',
fetchStatus: 'fetching',
data: undefined,
})
expect(states[1]).toMatchObject({
status: 'success',
fetchStatus: 'idle',
data: '',
})
expect(states[2]).toMatchObject({
status: 'success',
fetchStatus: 'fetching',
data: '',
})
expect(states[1]).toMatchObject({
status: 'success',
fetchStatus: 'idle',
data: 'phone',
})
}),
)On both the [
{ data: undefined, status: 'pending', fetchStatus: 'fetching' },
{ data: '', status: 'success', fetchStatus: 'idle' },
{ data: undefined, status: 'pending', fetchStatus: 'fetching' },
{ data: 'phone', status: 'success', fetchStatus: 'idle' }
]Which tbh is quite baffling; in So TL;DR I think it's likely you're right Theo -- we probably should use a thunk for params to preserve the observer instance between changes -- but I need to track down why the functioned version isn't working. |
|
Figured it out. Thanks for bearing with, all -- I'm struggling to piece together a full understanding of everything from a combination of the React docs, the Svelte docs, and the core code 😆 |
|
@elliott-with-the-longest-name-on-github really appreciate you taking the initiative on this and being so open and responsive with feedback! |
|
Hey, we're all just over here trying to figure out the best way to push bits around 🤷♂️ |
Unfortunately we are all in this boat when it comes to tanstack svelte docs / support. Thanks for all of your help. |
|
Just pushed changes:
|
|
Question to everyone (including you, @lachlancollins): Does anyone use I ask because:
TL;DR it seems like useless API surface area we could omit from the Svelte library unless we really need to keep it? Edit: I'm not even sure the internal implementation is correct? I think if you changed the value of In Svelte, for example, because the whole hook doesn't rerun anytime something changes, this pretends to work but doesn't really: return !resolvedOptions.notifyOnChangeProps
? observer.trackResult(query)
: queryBecause it only runs when the hook is set up -- so if $effect(() => {
if (resolvedOptions.notifyOnChangeProps) {
observer.clearTrackedProps();
resolvedOptions.trackedProps.forEach((prop) => observer.trackProp(prop))
} else {
// I'm not even sure it's possible to do this in a valid way, as you'd need to know which props are _currently_ being accessed, even though you never set up the listeners beforehand.
}
})From a very technical standpoint, we can just pretend this issue doesn't exist -- as long as you don't change the value of |
|
Can confirm the latest commits work great! I personally don't see a reason for |
|
For anyone who's technically inclined to care, the "one-level-deep raw state" implementation has evolved! This has turned into a really neat pattern for integrating Svelte with js-only packages. It started as "create a proxy from an initial object that turns any fields assigned to it into /**
* Makes all of the top-level keys of an object into $state.raw fields whose initial values
* are the same as in the original object. Does not mutate the original object. Provides an `update`
* function that _can_ (but does not have to be) be used to replace all of the object's top-level keys
* with the values of the new object, while maintaining the original root object's reference.
*/
export function createRawRef<T extends {} | Array<unknown>>(
init: T,
): [Readonly<T>, (newValue: T) => void] {
const refObj = (Array.isArray(init) ? [] : {}) as T
const out = new Proxy(refObj, {
set(target, prop, value, receiver) {
if (prop in target) {
return Reflect.set(target, prop, value, receiver)
}
let state = $state.raw(value)
Object.defineProperty(target, prop, {
configurable: true,
enumerable: true,
get: () => {
// If this is a lazy value, we need to call it.
// We can't do something like typeof state === 'function'
// because the value could actually be a function that we don't want to call.
return isBranded(state) ? state() : state
},
set: (v) => {
state = v
},
})
return true
},
})
function update(newValue: T) {
const existingKeys = Object.keys(out)
const newKeys = Object.keys(newValue)
const keysToRemove = existingKeys.filter((key) => !newKeys.includes(key))
for (const key of keysToRemove) {
// @ts-expect-error
delete out[key]
}
for (const key of newKeys) {
// @ts-expect-error
// This craziness is required because Tanstack Query defines getters for all of the keys on the object.
// These getters track property access, so if we access all of them here, we'll end up tracking everything.
// So we wrap the property access in a special function that we can identify later to lazily access the value.
// (See above)
out[key] = brand(() => newValue[key])
}
}
// we can't pass `init` directly into the proxy because it'll never set the state fields
// (because (prop in target) will always be true)
update(init)
return [out, update]
}
const lazyBrand = Symbol('LazyValue')
type Branded<T> = T & { [lazyBrand]: true }
function brand<T extends () => unknown>(fn: T): Branded<T> {
// @ts-expect-error
fn[lazyBrand] = true
return fn as Branded<T>
}
function isBranded<T>(fn: T): fn is Branded<T> {
return Boolean((fn as Branded<T>)[lazyBrand])
} |
| 'should be able to set different stale times for a query', | ||
| async () => { | ||
| /** | ||
| * TODO: There's a super weird bug with this test, and I think it's caused by a race condition in query-core. |
There was a problem hiding this comment.
@lachlancollins here's the mystery of the day, for which I have no solution. The only way to make it pass that I can find is to add a tight loop in query-core that delays execution of the rest of updateResult -- so I'm pretty confident this is a race condition -- I just can't for the life of me figure out where. I'm suspicious it's in query-core but I can't prove it...
|
FYI to anyone watching, if you end up with a couple of spare hours and want to help out, you can port some tests! I'm using the Solid ones because they're very similar to Svelte, and you can follow along with the |
There was a problem hiding this comment.
@elliott-with-the-longest-name-on-github when you get a chance, can you target the PR against TanStack:svelte-5-adapter? I've just rebased that branch onto TanStack:main (it was a few months behind), so there should hopefully be fewer conflicts.
elliott-with-the-longest-name-on-github
changed the base branch from
main
to
svelte-5-adapter
|
@lachlancollins apologies for the delay -- I'm actually prepping for my wedding so it's a hectic time 😆 I got it rebased and un-conflicted with your branch -- I think it's pretty well ready to go save for the tests -- there are still a bunch to port over from the Solid library. I'm hoping to be able to bash those out tomorrow and the day after. |
This is a proposal built on the excellent work of @lachlancollins. I've had a few conversations with Rich and the Svelte team about what the best user API is for libraries like this, and we think we've come up with a good solution. Basically, it's this:
There are several benefits to this:
FunctionedParamsrequireduseQueryThe main drawback is that, if the arguments change,
createQueryhas to be rerun, meaning the observer is recreated. Given that previously all of the observer's arguments had to be reapplied any time any one of them changed anyway, and the rate of change of arguments is typically slow (queryKeyisn't going to be updating hundreds of times per second, just... every once in a while), this tradeoff seems very worth it -- in fact, I haven't been able to observe any significant difference in performance.TODO:
Ignore everything under
examplesandintegrationsfor now, I'm working on getting them up to date