feat: Add SSR support#12
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive Server-Side Rendering (SSR) support across the framework, including document rendering on the server and hydration logic on the client. Key updates include SSR-specific bundler configurations, a new createSsrHandler for managing TanStack Router SSR, and enhanced transport logic to forward request context. Feedback identifies a critical issue where the AsyncLocalStorage instance needs to be a singleton to prevent context loss across multiple bundles and suggests a more robust method for handling multi-value headers during record conversion.
| const transportRequestContext = new AsyncLocalStorage<{ | ||
| baseUrl?: string; | ||
| headers?: Record<string, string>; | ||
| }>(); | ||
|
|
||
| (globalThis as Record<symbol, unknown>)[REQUEST_CONTEXT_KEY] = | ||
| transportRequestContext; |
There was a problem hiding this comment.
The AsyncLocalStorage instance should be a singleton across the entire process to ensure that request context is correctly shared even if multiple versions or bundles of @evjs/server are loaded. Currently, every time this module is evaluated, it creates a new instance and overwrites the global symbol, which will cause getStore() to return undefined for code running in a previously established context from a different instance.
const transportRequestContext = (function () {
const globalAny = globalThis as any;
if (!globalAny[REQUEST_CONTEXT_KEY]) {
globalAny[REQUEST_CONTEXT_KEY] = new AsyncLocalStorage<{
baseUrl?: string;
headers?: Record<string, string>;
}>();
}
return globalAny[REQUEST_CONTEXT_KEY] as AsyncLocalStorage<{
baseUrl?: string;
headers?: Record<string, string>;
}>;
})();| function headersToRecord(headers: Headers): Record<string, string> { | ||
| const result: Record<string, string> = {}; | ||
| headers.forEach((value, key) => { | ||
| if (!FORWARDED_HEADER_DENYLIST.has(key.toLowerCase())) { | ||
| result[key] = value; | ||
| } | ||
| }); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Using headers.forEach to build the record might not correctly handle headers with multiple values, as it may overwrite previous values or use an incorrect separator depending on the environment's Headers implementation. Using headers.get(key) for each unique key is generally safer as it returns the correctly combined value according to the Fetch spec.
function headersToRecord(headers: Headers): Record<string, string> {
const result: Record<string, string> = {};
const keys = new Set(headers.keys());
for (const key of keys) {
if (!FORWARDED_HEADER_DENYLIST.has(key.toLowerCase())) {
const value = headers.get(key);
if (value !== null) {
result[key] = value;
}
}
}
return result;
}# Conflicts: # packages/client/src/app.tsx # packages/client/src/transport.ts
Summary
@evjs/server/ssr, including route-tree helpers, document rendering, asset tags, and transport request-context forwarding.@evjs/client, while keeping RSC hidden from public docs and exports.Validation
npm test -w @evjs/clientnpm test -w @evjs/servernpm test -w @evjs/evnpm test -w @evjs/bundler-utoopacknpm run check-typesnpm run test:e2e -- e2e/cases/ssr.ts --project=utoopacknpm run lint