Skip to content
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: stream non-essential data #8901

Merged
merged 66 commits into from
Feb 20, 2023
Merged

feat: stream non-essential data #8901

merged 66 commits into from
Feb 20, 2023

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Feb 6, 2023

Closes #7213

This implements the proposed defer utility (inspired by React Router). At the moment this only works for client-side navigations. To implement this for SSR I see a couple of options. They all start with streaming the initial HTML which contains _deferred_X strings in place where the promises are that are still pending. The options revolve around how the chunks look like that come in later streaming for server load functions which you can utilize in the same way you can today with +page.js load functions by nesting the promise.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

@dummdidumm
Copy link
Member Author

@Rich-Harris brought up the nice idea of moving parts of this into devalue. devalue could take a custom serialize function, which we use to tell it to transform promises to something like ['Promise', 'id_of_the_promise'] - that way we also don't need to worry about our id strings clashing with anything the user might've written. It would also enable us to defer promises at any level in the returned object, which is a nice QOL feature. Theoretically that makes the defer functions obsolete since you could move promises you want to defer into a nested object, but the ergonomics of that are not that nice IMO so I would be favor of keeping it. For v2 we could think about switching the logic around: top level promises are not auto-awaited, instead they function like if they were wrapped with defer.

@Rich-Harris
Copy link
Member

PR to devalue: Rich-Harris/devalue#58

@Rich-Harris
Copy link
Member

I think it might be weird to introduce a defer function whose job is to not transform its input (whereas normally the returned object is transformed).

I'd personally find this...

export function load({ fetch }) {
  return {
    post: fetch(url).then(r => r.json()),
    lazy: {
      comments: fetch(url).then(r => r.json())
    }
  };
}
<script>
  export let data;
</script>

<div>{@html data.post}</div>
{#await data.lazy.comments}
  <p>loading comments...</p>
{:then comments}
  <!-- TODO -->
{:then error
  <!-- TODO -->
{/await}

...more natural than this:

import { defer } from '@sveltejs/kit';

export function load({ fetch }) {
  return defer({
    post: await fetch(url).then(r => r.json()),
    comments: fetch(url).then(r => r.json())
  });
}

Case in point: it's not until after I wrote that example that I realised it contains a waterfall.

If we do feel like awaiting top-level promises is confusing/a mistake and decide to change it in v2, then we could always add a config setting that opts you into v2 behaviour — we've done that for Svelte in the past. Then you'd just have to be careful about avoiding waterfalls:

export function load({ fetch }) {
  return {
    comments: fetch(url).then(r => r.json()), // streamed, because no `await`
    post: await fetch(url).then(r => r.json()) // ready on render, because `await`
  };
}

@Rich-Harris
Copy link
Member

Released devalue@4.3.0 with custom types support

@dummdidumm
Copy link
Member Author

Nice!
Note for monday: Keeping track of which resolved promise belongs to which node is super complicated. In the current design it would be needed to send along the final uses object. But since it's so complicated I think it's easier to send on final { type: uses, nodes: [{ url: 1, ..}, ..] } chunk once everything else is completed.

@Rich-Harris
Copy link
Member

I'm not sure I follow — why do we need to track that? Surely all that matters is the promise id?

@dummdidumm
Copy link
Member Author

dummdidumm commented Feb 10, 2023

If we stream a promise we don't know if all the uses fields are final yet. After a promise is resolved someone could access params, which only then are marked as used (if not accessed before). So we need to delay (or send again) the uses value after all promises of a load are resolved

@Rich-Harris
Copy link
Member

I think it would be totally fine if we said that you need to access those things before returning from load — I don't think I would have expected it to work differently as a user. Maybe we try shipping without that and see if it turns out to be an issue in practice?

@paoloricciuti
Copy link
Member

I'm commenting on this because I've implemented a version of defer in user land. I would probably publish this library soon but I will be more than happy to deprecate it when this land stable in SK.

Currently actual streaming is not supported in the load function so I've had to work around this by using a dedicated endpoint with server sent events but I think I found a decent enough solution (would love some insight on this from some other maintainers, for the moment I've talked about this to Geoff and Antony.

So basically here's how my library works, you have to add a provided handle function in hooks.server.ts, than in your +page.server.ts you wrap the load function inside a provided defer function. Finally you access the data through a store (so not with export let data). And that's it.

  • The handle function add two endpoints to your project, a GET and a POST: the GET endpoint uses server sent event to notify the client whenever one of the Promises on the server has resolved and the POST endpoint is used to trigger such event
  • The defer function takes the return value of your load function and keep track of your Promises sticking them into an array.
  • Once on the client the store create an EventStream and register a callback on the server sent event, it take the data returned from the load function and for every key in the promises array it create a new Promise saving the resolver and the rejecter in a Map
  • Once the promise on the server resolves a post request is made to the POST endpoint managed by the handle function
  • A new event is put out from the server sent event endpoint
  • The callback registered in the store fires and the store resolves the correct Promise giving you back the data.

From a security standpoint the first time you call a load function it generates a secure cookie with a random UUID that uses to just get your events from the server.

Here's the link to the repo if you want to take a look:

https://github.com/paoloricciuti/sveltekit-defer

Currently this works fine with adapter node.

@Suyashtnt
Copy link

How will defer work if the client doesn't have JS enabled?

@paoloricciuti
Copy link
Member

How will defer work if the client doesn't have JS enabled?

I don't think it could ever work...you have to await a promise on the client for defer to have sense so no-js = no ability to await promises.

@Suyashtnt
Copy link

How will defer work if the client doesn't have JS enabled?

I don't think it could ever work...you have to await a promise on the client for defer to have sense so no-js = no ability to await promises.

Maybe allow an option which makes defer only run on client side navigation. On SSR, it resolves on the server. If it's client side navigation, defer well, defers.

@Conduitry
Copy link
Member

The problem there is that all SSR in Svelte is currently synchronous, and you can't synchronously tell whether a promise has already resolved. So you would need to write your component in a way that it can accept either direct data or a promise resolving to data. At that point, if you're already doing that, you may as well just also write your load function so that it checks whether you're on the browser and then either conditionally calls defer or awaits the promise itself.

@david-plugge
Copy link
Contributor

I'm pretty sure this would work:

function deferClient(data) {
  return ssr ? data : defer(data)
} 

@Rich-Harris
Copy link
Member

To be clear, deferring will only apply to server loads, not universal loads. If you return an object containing a promise from a universal load function, its resolved value will be discarded and the promise will be created anew when the universal load function runs in the browser. (You can do this today, by the way — it's just how universal load functions work.)

So you wouldn't be detecting whether you're in the browser or not, you'd be detecting whether the server load function is running for SSR or for client-side navigation:

// src/routes/whatever/+page.js
export function load({ isDataRequest }) {
  return {
    criticalData: getCriticalData(),
    lazyData: {
      whatever: isDataRequest ? getLazyData() : await getLazyData()
    }
  }
}

In your component, this would work whether the data was awaited or not:

{#await data.lazyData.whatever}
  loading...
{:then whatever}
  {whatever}
{:catch error}
  {error.message}
{/await}

To be clear, I'm not necessarily suggesting you actually do this, just that you can.

@Smirow
Copy link

Smirow commented Feb 20, 2023

Maybe a stupid remark; I am guessing this should not work in +layout.server.ts? (It's not explicit in the docs).
And if it expected to work, what is the behavior of await parent()?

@Rich-Harris
Copy link
Member

It'll work in +layout.server.ts just the same. await parent() works the same way it does today — top-level promises are awaited, but other than that it just returns the object returned from the parent load

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

option to render layout before page is loaded
8 participants