feat: seroval json mode #2042
feat: seroval json mode #2042
Conversation
🦋 Changeset detectedLatest commit: db4c511 The changes in this PR will be included in the next version bump. 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 |
✅ Deploy Preview for solid-start-landing-page ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
commit: |
| middleware?: string; | ||
| serialization?: { | ||
| // This only matters for server function responses | ||
| mode?: 'js' | 'json'; |
There was a problem hiding this comment.
When possible, add a plugins option here that we can load into the serializer. The only problem is how do we bridge it from the config to the runtime (do we need a "runtime" config API?)
There was a problem hiding this comment.
The non-magical way I could think of is to add this as an option to createHandler in entry-server.tsx 🤔
There was a problem hiding this comment.
either that or an "entry" file for configs I suppose.
There was a problem hiding this comment.
Wouldn't we be able to set an environment variable, then use something like
const serialization = import.meta.env.VITE_SOLIDSTART_SERIALIZATION;
if (["js", "json"].includes(serialization ?? "js")) {
// initialize seroval mode
} else if (serialization) {
const [serializer, deserializer] = await import(serialization);
}There was a problem hiding this comment.
@atk Not exactly this API in mind. I'm not trying to override seroval, I'm more of trying to think of an idea to allow incorporating 3P seroval plugins
| URLSearchParamsPlugin, | ||
| URLPlugin, | ||
| ]; | ||
| const MAX_SERIALIZATION_DEPTH_LIMIT = 64; |
There was a problem hiding this comment.
Seroval has a default limit of 1000, but that's because of my poor judgment. Will probably port this default back.
| // TODO(Alexis): move to serializeToJSONStream | ||
| body: await serializeToJSONString(args), | ||
| // duplex: 'half', | ||
| // body: serializeToJSONStream(args), |
There was a problem hiding this comment.
bleeding edge. TypeScript doesn't have the duplex property defined yet so this one is invalid for now. On top of that, this doesn't actually "stream" as in the server responds immediately on request. What the browser does currently is buffer the entire stream then sends in bulk (technically speaking, duplex: half describes this behavior, duplex: full describes the desired behavior).
Once all browsers are ready for this (and also TypeScript), we can swap to duplex: full
| }); | ||
| } else if (contentType?.startsWith('application/json')) { | ||
| parsed = await event.request.json() as any[]; | ||
| } else if (request.headers.has('x-serialized')) { |
There was a problem hiding this comment.
- We no longer use the
application/jsonmime type for seroval - We wanted to distinguish seroval-based body from JSON body. Did I just add a new feature?
Pretty straight forward. Adds an opt-in mode to use json mode (set by default) for seroval streaming, this is because the js stream is affected by CSP.
Also adds streaming support for client-to-server communication (except for url-encoded requests), assuming that the browser it comes from supports it.