skip pending block for already-resolved promises on mount #11995
Conversation
🦋 Changeset detectedLatest commit: 9988752 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 |
|
|
||
| if (catch_fn) { | ||
| catch_effect = create_effect(catch_fn, error); | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| Promise.resolve().then(() => { |
There was a problem hiding this comment.
This should use something that plays nicely with flushSync - queue_micro_task for example. Else this makes mount async (conceptually).
In general I don't think this is a case worth optimizing for:
- SSR is sync, so you can't set a promise into settled state right away. That means before hydration you see the pending state anyway, not worth adding extra logic to then "repair" the dom because it's in settled state now
- mount should be sync conceptually (we agreed on this recently), so it doesn't really make sense there either
There was a problem hiding this comment.
I view this as a correctness issue as much as an optimisation issue. In mount we're currently rendering a pending block for a promise that isn't pending. The fact that we're doing that and then destroying it before the user even sees it makes that worse, not better.
queue_micro_task doesn't help here, because the work can't happen before the original promise handlers have had a chance to run.
There was a problem hiding this comment.
but does it matter in practise? Is this something worth considering? Who would have a resolved promise on mount? Even switching between resolved promises is an edge case, a resolved promise on mount even moreso.
There was a problem hiding this comment.
You could imagine a case where a SvelteKit load function does this...
export function load() {
return {
critical: 'some critical data',
streamed: get_promise()
};
}...and the promise resolves before hydration begins. Or a case where you have a client-side cache. I don't think these are the majority of cases, but I don't think they're edge cases either
|
closing in favour of #12274 |
#11989 got me thinking — aside from the question of keeping
then/catchblocks mounted rather than recreating them each time (as though they contained{#key ...}blocks), why do we create pending blocks at all on mount, if the promise is already resolved?This PR suggests an approach to that
(though it appears to have picked up some unrelated changes due to a sloppy merge, oops). More intended to spark discussion than anything — it's draft for two reasons:Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.Tests and linting
pnpm testand lint the project withpnpm lint