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

Make a sveltekit-ssr example (universal and server-only) #605

Closed
wants to merge 5 commits into from

Conversation

dimitropoulos
Copy link
Contributor

This PR implements a sveltekit example demonstrating how to do SSR in both styles that sveltekit supports (both of Universal and Server-only).

Copy link
Collaborator

@paul-sachs paul-sachs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great example! I wish we had a better CRUD service to better show server side rendering but it works!!!

svelte/sveltekit-ssr/src/routes/server-ssr/+page.svelte Outdated Show resolved Hide resolved
svelte/sveltekit-ssr/src/routes/universal-ssr/+page.svelte Outdated Show resolved Hide resolved
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, but I don't think the universal load function is working. The log statements show up in the browser and in the console, so the function is executed on the server and on the frontend, which is expected.

But it is issuing a HTTP request (with npm start but also with npm run build && npm run preview):

Screenshot 2023-05-22 at 09 52 09

I would expect svelte to record the response on the server side, and embed it in the web page. When the document loads in the browser, I would expect svelte to run the load function and hydrate the fetch response from the embedded data instead of issuing a HTTP request.

If this does not work, it would be nice to understand why, and whether there is a reasonable way to work around it for the user or for us. We should not give an example that does not work as expected.

For the server load function, we rely on page data to be serialized and parsed again. What are the constraints with protobuf here? I imagine that bigint and classes might not be well supported.

@dimitropoulos
Copy link
Contributor Author

dimitropoulos commented May 22, 2023

from the embedded data instead of issuing a HTTP request

what gives you this expectation?

What I see in the docs are

Now, the functions will run on the server during server-side rendering, but will also run in the browser when the app hydrates or the user performs a client-side navigation.

and

By default, universal load functions run on the server during SSR when the user first visits your page. They will then run again during hydration, reusing any responses from fetch requests.

@timostamm
Copy link
Member

With a plain fetch, using Response.json(), I see hydration data in the page source, and no additional request from the client.

+page.ts
import { SayRequest } from '../../gen/buf/connect/demo/eliza/v1/eliza_pb'
import type { PageLoad } from './$types'
export const load: PageLoad = async ({ fetch }) => {
  const r = await fetch("https://demo.connect.build/buf.connect.demo.eliza.v1.ElizaService/Say", {
    method: "POST",
    headers: {
      "Content-Type": "application/json"
    },
    body: new SayRequest({
      sentence: "hi from the server",
    }).toJsonString()
  });
  const body = await r.json();
  console.log("server sentence:", body.sentence)
  return { universalSentence: body.sentence }
}
Network inspector w/o additional requests Screenshot 2023-05-22 at 18 08 50
Hydration data in the document
	<script type="application/json" data-sveltekit-fetched data-url="https://demo.connect.build/buf.connect.demo.eliza.v1.ElizaService/Say" data-hash="1y6nxzi">{"status":200,"statusText":"OK","headers":{"content-type":"application/json"},"body":"{\"sentence\":\"Hi there...how are you today?\"}"}</script></div>
    </body>
</html>

@timostamm
Copy link
Member

The SSR example in #610 turned out nice and simple. I think we should take the same approach here, specifically:

  1. add the server-only SSR example as a separate page inside the existing project instead of adding the new one, and do only SSR on the page
  2. show passing a protobuf message and passing individual properties in the server-only example, like we did for getServerSideProps
  3. add the universal-load example a separate page in the existing project too, but pass the protobuf message (class instance) through without the toJson workaround, because that should work with universal-load
  4. add a navigation to switch between the pages

Regarding the example for universal load, I still do not believe that it actually works. I think we might be able to make a change in @bufbuild/connect-web specifically to support it, and thereby also helping out with #602 and connectrpc/connect-es#199. I'm cool with merging it anyhow so that we have a reproducible test case, it just needs a TODO explaining that it isn't currently supported and that we are looking into it. If it turns out that is not feasible, we can still remove the example again.

@timostamm
Copy link
Member

Superseded by #674

@timostamm timostamm closed this Jun 5, 2023
@smallsamantha smallsamantha deleted the dimitri/svelte-ssr branch September 18, 2023 13:54
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.

3 participants