-
Notifications
You must be signed in to change notification settings - Fork 173
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
Next.js SSR example #1596
base: main
Are you sure you want to change the base?
Next.js SSR example #1596
Conversation
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.
Left a few comments.
Don't you want to use either https://nextjs.org/docs/pages/building-your-application/data-fetching/get-server-side-props or https://nextjs.org/docs/pages/building-your-application/data-fetching/get-static-props here? Right now I don't see that any data is being cached during server rendering.
EDIT: Oh I see... this is RSC stuff
64f3703
to
2d933e8
Compare
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
First, I think there is going to be two ways people want to do SSR with
I think we want to provide tooling for both of these cases. On 1 - snapshot of the shape and resumeWhile the pattern in this PR works, having to pass the shape snapshot to the component using I think this is doable if we create a We are somewhat restricted as we cant use a react context, but something like this POC is possible: https://github.com/samwillis/ssr-shape-poc (I've purposely not used any electric, just wanted to test the concept for injection) By the user adding This is also not tied to Next, and should work with all React SSR frameworks. On 2 - a minimal subset on the serverI think we should add a useShape({
url: `http://localhost:3000/v1/shape/foo`,
getServerSnapshot: () => {
// Do and return a Postgres query
},
}) |
Great stuff @samwillis :) On 2, I wonder if there's a way of supporting |
02ffd0d
to
dd64a75
Compare
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.
@samwillis inspired on your example and Nextjs docs, I gave another try. The idea here is to have a context around the component that takes the server-sent data from props and initializes the shapes on the client --- totally transparent on the client component!
Left some comments
@samwillis, @thruflo my initial idea was to call the database directly on the server. This is a powerful pattern to show, but because of the problems with ordering and paging, I thought it would be better to build just on Shapes. The benefit of Shapes is that requests go through the cache and doesn't put load on the server db. A naive way of rendering the subset of data on the server is to just drop the data that doesn't fit into the initial page rendering :) |
887f224
to
2d8330e
Compare
@samwillis , I did a DX refactoring addressing your concerns with it being simple to specify the shapes across server and client. Have a look at it or ping me to see together. I also want to replace the base nextjs example with this one (cc @KyleAMathews) |
0eccfa5
to
1b3880d
Compare
1b3880d
to
dc9da79
Compare
Examples |
@@ -62,7 +62,7 @@ jobs: | |||
|
|||
|
|||
- name: Deploy NextJs example | |||
working-directory: examples/nextjs-example | |||
working-directory: examples/nextjs-ssr-example |
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.
Just a naming nit: there's no need to suffix with -example
, that's communicated by the parent folder name, so I would suggest examples/nextjs-ssr
.
@@ -0,0 +1,22 @@ | |||
# Next.js SSR example |
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.
Generally, the README for an example should ideally communicate a bit about what it's trying to show you, i.e.: what is the example, why is it interesting, what can you achieve with the pattern it demonstrates, etc. Plus perhaps some signposts into key parts of the code.
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.
Will work on that when we decide to publish
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.
We're purple nowadays/
} from '@electric-sql/client' | ||
import React from 'react' | ||
import { useSyncExternalStoreWithSelector } from 'use-sync-external-store/with-selector.js' | ||
|
||
type UnknownShape = Shape<Row<unknown>> | ||
type UnknownShapeStream = ShapeStream<Row<unknown>> | ||
export type SerializedShape = { | ||
offset: Offset | ||
shapeHandle: string | undefined |
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.
Another very minor nit but seeings how this is in the react-hooks library not just the example, I've been trying to move us towards just using handle
rather than shapeHandle
.
Below when you getSerialisedShape you can also just read it from shape.handle
rather than shapeStream.shapeHandle
.
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.
yep, i started this a while back, things like the offset were not public at the time. Will cleanup.
const shape = getShape(shapeStream) | ||
return { | ||
shapeHandle: shapeStream.shapeHandle, | ||
offset: shapeStream.offset, |
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.
Does offset
work? Is the property not lastOffset
?
@@ -245,6 +245,10 @@ export class ShapeStream<T extends Row<unknown> = Row> | |||
return this.#shapeHandle | |||
} | |||
|
|||
get offset() { |
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.
I think this is duplicated now -- we already expose lastOffset as a public getter.
Items example using SSR.
Load the entire shape on the server and send it down to the client, so that the client gets the initial state of the shape on first response and can resume replication from the tip of the shape stream.
First time with nextjs, I still don't know how some things work. I suppose this is good forn an example. I'm happy to receive feedback from nextjs experts :D