-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: async SSR #14447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: async SSR #14447
Conversation
🦋 Changeset detectedLatest commit: 18051e1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
import root from '../root.${isSvelte5Plus() ? 'js' : 'svelte'}'; | ||
import { set_building, set_prerendering } from '__sveltekit/environment'; | ||
import { set_assets } from '__sveltekit/paths'; | ||
import { set_assets } from '$app/paths/internal/server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems pretty close $app/paths
and i'm getting flashbacks from libraries messing with svelte runtime internals.
Do we have to prevent users from accessing internal somehow? maybe $internal would be one more step away from $app to make it clearer? Or is there a case for import maps here?
I do like the replacement of __sveltekit which struck me as odd before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same as svelte/internal
— accessible, but disallowed by types. I don't believe it's possible to not make it publicly accessible to someone who really wants to shoot themselves in the foot, but I also don't think that's a real problem. It won't show up as an auto-import option, and if you do import */internal
in user code you'll be rewarded with a red squiggly
Btw, with this PR, the // edit: Aaaand it has been fixed?! 🙌 |
Amazing! |
I was trying to figure out if the following is achievable with the current async SSR / remote functions / svelte:boundary setup: On initial load, certain blocks should be SSR’d (for SEO reasons), but on client-side navigation we’d prefer to quickly show skeletons instead of blocking. svelte:boundary feels like the right tool for this, but those aren't SSR'd. I’ve experimented with conditionally awaiting remote functions based on whether navigation.complete is null or not, but error handling becomes pretty tricky that way. Is there currently a good pattern for this kind of “SSR once, skeleton on client navigation” pattern, or is this something the framework might eventually provide an easier path for? |
@amit13k I'm pretty sure when you have a boundary the pending snippet is SSR'd so you'd use that Edit: Oh I think I actually understand what you meant: I think this actually is where |
Right now, no. The same One idea we've talked about is having a flag on the boundary — something like this: <svelte:boundary ssr="block">
<!-- ... -->
{#snippet pending()}
<p>loading</p>
{/snippet}
</svelte:boundary> This would allow you to explicitly say 'ignore the fact that this boundary has a pending snippet, I want to SSR the contents'. I did think something like this might work... {#snippet pending()}
<p>loading...</p>
{/snippet}
<svelte:boundary pending={typeof window !== 'undefined' ? pending : null}>
<p>{await delayed('hello')}</p>
</svelte:boundary> ...but it looks like we incorrectly interpret the presence of a |
Would this just be checking for |
Basically, yeah. If it's a |
This adds asynchronous SSR to an app that's using the latest version of Svelte (
^5.39.3
) and has opted in to the Svelteexperimental.async
option (and, ideally, the SvelteKitexperimental.remoteFunctions
option):This means that it's possible to use
await
anywhere in your app, without wrapping it in a boundary with apending
snippet. SSR will wait for all such 'naked'await
expressions to resolve.In turn, this means that it's practical to use remote functions, as it no longer forces you to put everything behind skeleton UI:
Any remote function calls that occur during SSR will have their return values serialized and reused upon hydration, without an additional network round trip.
There are a few subtle bugs and caveats that make this unsuitable for production for the time being:
{#if await condition}
block or something) then it doesn't pull the value from the serialized data. The data is there, we're just not using it in that situation. Need to figure out how to do so while guaranteeing freshness.remote.ts
file, it will have an incorrect cache key and the serialized data will be incorrectly sent to the client (this is an easy fix, will work on it next)<svelte:boundary>
will update together, but in the current implementation things will update as they are 'discovered'. In most cases, this does not matter, but it's observable if you have (for example) a{Date.now()}
in multiple placesload
functions to call and calling them. There is, as yet, no equivalent for inlineawait
expressions, because Svelte does not have an API for 'forking'All these things will be fixed in due course.
Note to reviewers: other than the obvious changes in
render.js
, the big difference in this PR is that the virtual__sveltekit/paths
module is no more. That's because we need different implementations ofresolve(...)
andasset(...)
between client and server. On the server, we use the event store to determine the currentevent.url.pathname
, so that we can construct a relative pathname (something that previously worked by settingbase
andassets
immediately before render, and unsetting immediately afterwards) even if they are called after anawait
. On the client, this is unnecessary.This change is long overdue. The virtual modules are a scourge, and I'd like to get rid of all of them. Having type safety and go-to-definition and everything else that goes with it is just so much nicer. All we need to do is have a few more
define
constants.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits