feat: hydratable and a more consistent remote functions model#15533
feat: hydratable and a more consistent remote functions model#15533elliott-with-the-longest-name-on-github wants to merge 10 commits intomainfrom
hydratable and a more consistent remote functions model#15533Conversation
|
packages/kit/src/runtime/client/remote-functions/prerender.svelte.js
Outdated
Show resolved
Hide resolved
packages/kit/src/runtime/client/remote-functions/query.svelte.js
Outdated
Show resolved
Hide resolved
…rowser consoles
This commit fixes the issue reported at packages/kit/src/runtime/client/remote-functions/query.svelte.js:58
**Bug Explanation:**
Three debug console.log statements were left in the production code at:
* Line 58: `console.log(cache_key + ' bypassed hydratable');`
* Line 292: `console.log(this._key, 'serving from the cache');`
* Line 295: `console.log(this._key, 'made it past the cache');`
These statements appear to be development debugging aids that were not removed before committing. The codebase has a clear pattern for console.log usage:
1. In build/CLI tools (acceptable since those run in Node.js during development)
2. Guarded by `DEV` checks for runtime code (e.g., line 671 in respond.js uses `if (DEV && ...) { console.log(...) }`)
The unguarded console.log statements in query.svelte.js would execute on every query run/cache check in production, spamming end users' browser consoles with internal debugging messages like "query-key serving from the cache" and "query-key bypassed hydratable".
**Fix:**
Removed all three debug console.log statements since they serve no purpose for production users and were clearly left in accidentally (evidenced by commit messages like "i think it work" indicating development-in-progress code).
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: elliott-with-the-longest-name-on-github <hello@ell.iott.dev>
packages/adapter-netlify/test/apps/split/src/routes/remote/query/+page.svelte
Show resolved
Hide resolved
| * Unlike awaiting the resource directly, this can be used anywhere | ||
| * (load functions, event handlers, etc) without requiring a reactive context. |
There was a problem hiding this comment.
we'd talked about making it not work in effects — is that the plan? (or already implemented, though i'll figure that out by carrying on reading) if so this comment will need to change
There was a problem hiding this comment.
not implemented. though I'm still ambivalent. I don't think it would be too hard to implement and it might be a good idea.
| start: () => get_response(__, arg, state, get_remote_function_result), | ||
| refresh: () => get_remote_function_result() |
There was a problem hiding this comment.
we can simplify this signature
| start: () => get_response(__, arg, state, get_remote_function_result), | |
| refresh: () => get_remote_function_result() | |
| get_remote_function_result |
| 'x-sveltekit-search': page.url.search | ||
| })); | ||
| return untrack(() => { | ||
| const url = navigating.current?.to?.url ?? page.url; |
There was a problem hiding this comment.
this feels a little brittle — you could navigate, triggering a load function that will eventually call a query, then navigate again before it actually happens. not yet sure what the right way to think about it is
This PR makes a number of substantial changes to how remote queries work.
hydratableImplementation-wise, it replaces our custom transport solution with
hydratablefor queries that are used during render -- this method of transport is more correct and prevents us from accidentally using stale data for queries in a small subset of cases. There's one breaking side effect of this: If you didn't render a query on the server, you can't render it during hydration.It means this:
would throw during hydration because
get_counttried to access cached data that did not exist. This is almost always an undesirable mistake: It means you introduced a waterfall on the client and blocked hydration. If you really do want this, you'd do something like this instead:New rules around query usage
From now on, on the client, you can only access a query's data if that query is:
It may be easier to illustrate when you can't create and use a query on the client:
loadWe're making this change because otherwise it's basically impossible to reliably cache queries across your app. However, for most use cases, this probably doesn't cause any pain:
.refresh,.set,.withOverrideare all availablequery().run(), which will return a plain oldPromiseresolving to your dataBugfixes
$effect.rootwhen created. This prevents them from associating with their parent effect, making their usage more predictable when retrieved from the cache.refreshis now a noop if there is no cached instance of the query, meaning you won't have a useless query request in some circumstanceshydratable!)