Skip to content

feat: Support SSR, SSG and HMR#10

Draft
dawsontoth wants to merge 1 commit into
mainfrom
production
Draft

feat: Support SSR, SSG and HMR#10
dawsontoth wants to merge 1 commit into
mainfrom
production

Conversation

@dawsontoth
Copy link
Copy Markdown
Contributor

@dawsontoth dawsontoth commented Jun 3, 2026

This adds support for this plugin to run in production, with or without SSR enabled too. This can let us run Vite based apps in Studio, in theory.

I created new templates in create-harper over at HarperFast/create-harper#98

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the Harper-Vite integration, introducing a hybrid production mode that statically serves production builds and supports automatic recompilation alongside the existing development mode. It also adds support for Server-Side Rendering (SSR), coordinates multi-threaded builds using a file-system lock, and introduces comprehensive tests. The review feedback identifies several critical issues: a race condition in the build lock release check, a severe memory leak in registerHttp due to unresolved promises on successful middleware responses, a downtime window during SSR rebuilds caused by hardcoded directory emptying, performance bottlenecks from synchronous file operations in HTTP handlers, and a potential double-close bug in registerShutdown.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/buildLock.ts Outdated
Comment thread src/http.ts Outdated
Comment on lines +17 to +32
export function registerHttp(scope: any, middleware: Middleware): void {
if (!scope?.server?.http) return;
scope.server.http(
(request: any, nextLayer: (request: any) => unknown) =>
new Promise((resolve, reject) => {
middleware(request._nodeRequest, request._nodeResponse, (err?: unknown) => {
if (err) return reject(err);
try {
resolve(nextLayer(request));
} catch (e) {
reject(e);
}
});
})
);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In registerHttp, the returned Promise is only resolved when the middleware calls next(). However, Connect-style middlewares (such as sirv or Vite's middleware) do not call next() when they successfully handle a request and send a response. This causes the Promise to remain pending forever for almost all static asset and HTML requests, leading to a severe memory leak under production load.

We should listen to the response's finish and close events to resolve the Promise and clean up listeners when the response is completed.

export function registerHttp(scope: any, middleware: Middleware): void {
	if (!scope?.server?.http) return;
	scope.server.http(
		(request: any, nextLayer: (request: any) => unknown) =>
			new Promise((resolve, reject) => {
				const res = request._nodeResponse;
				
				const cleanup = () => {
					res.removeListener('finish', onFinished);
					res.removeListener('close', onClosed);
				};
				
				const onFinished = () => {
					cleanup();
					resolve(undefined);
				};
				
				const onClosed = () => {
					cleanup();
					resolve(undefined);
				};
				
				res.on('finish', onFinished);
				res.on('close', onClosed);

				middleware(request._nodeRequest, res, (err?: unknown) => {
					cleanup();
					if (err) return reject(err);
					try {
						resolve(nextLayer(request));
					} catch (e) {
						reject(e);
					}
				});
			})
	);
}

Comment thread src/production.ts
Comment on lines +51 to +56
if (ssrEntry) {
await viteWrapper.build({
root,
build: { ssr: ssrEntry, outDir: serverOutDir, emptyOutDir: true },
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The SSR build has emptyOutDir: true hardcoded. This means every automatic rebuild will delete the entire serverOutDir before compilation starts. During this compilation window (which can take several seconds), any incoming SSR request will fail with ENOENT or module resolution errors because the server entry file is missing. This introduces a downtime window, contradicting the goal of a zero-downtime atomic swap.

To achieve a truly zero-downtime swap, consider building to a temporary directory and renaming it upon success, or at least set emptyOutDir: false to avoid deleting the existing bundle before the new one is ready.

Suggested change
if (ssrEntry) {
await viteWrapper.build({
root,
build: { ssr: ssrEntry, outDir: serverOutDir, emptyOutDir: true },
});
}
if (ssrEntry) {
await viteWrapper.build({
root,
build: { ssr: ssrEntry, outDir: serverOutDir, emptyOutDir: false },
});
}

Comment thread src/production.ts Outdated
Comment on lines +119 to +122
const template = readFileSync(join(clientOutDir, 'index.html'), 'utf-8');
// Cache-bust the import by the bundle's mtime so every worker picks up a rebuilt SSR bundle,
// regardless of which worker performed the build.
const version = statSync(serverEntryPath).mtimeMs;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Using synchronous file system operations (readFileSync and statSync) inside the HTTP request handler blocks the single-threaded Node.js event loop on every single HTML request. Under production load, this will severely limit the throughput of the server and increase response latency.

Consider using asynchronous file system operations (fs.promises.readFile and fs.promises.stat) or, even better, caching the index.html template in memory since it only changes when a rebuild completes.

Comment thread src/http.ts Outdated
Comment on lines +35 to +46
export function registerShutdown(scope: any, close: () => unknown): void {
const shutdownHandler = (msg: any) => {
if (msg?.type === 'shutdown') close();
};

scope.on('close', () => {
close();
parentPort?.off('message', shutdownHandler);
});

parentPort?.on('message', shutdownHandler);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In registerShutdown, if a shutdown message is received via parentPort, close() is called. However, if the scope later emits close, close() will be called a second time. To prevent potential errors or warnings from double-closing resources (like the Vite server), we should ensure close() is only executed once.

Suggested change
export function registerShutdown(scope: any, close: () => unknown): void {
const shutdownHandler = (msg: any) => {
if (msg?.type === 'shutdown') close();
};
scope.on('close', () => {
close();
parentPort?.off('message', shutdownHandler);
});
parentPort?.on('message', shutdownHandler);
}
export function registerShutdown(scope: any, close: () => unknown): void {
let closed = false;
const safeClose = () => {
if (closed) return;
closed = true;
close();
};
const shutdownHandler = (msg: any) => {
if (msg?.type === 'shutdown') safeClose();
};
scope.on('close', () => {
safeClose();
parentPort?.off('message', shutdownHandler);
});
parentPort?.on('message', shutdownHandler);
}

Copy link
Copy Markdown
Member

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

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

I like the idea, but the implementation needs some work. I assume an agent created this. Could you potentially have it review the Next.js plugin so it can see some better development patterns?

Comment thread scripts/setup-fixture.mjs
Comment on lines +12 to +15
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';

const root = join(dirname(fileURLToPath(import.meta.url)), '..');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
const root = join(dirname(fileURLToPath(import.meta.url)), '..');
import { join } from 'node:path';
const root = join(import.meta.dirname, '..');

Comment thread src/buildLock.ts
Comment thread src/buildLock.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/wrappers.ts Outdated
Comment thread src/production.ts Outdated
Comment thread test-fixture/config.yaml
Comment thread src/index.ts Outdated
Comment thread src/http.ts Outdated
Comment thread src/http.ts Outdated
@dawsontoth dawsontoth marked this pull request as draft June 3, 2026 17:30
@dawsontoth
Copy link
Copy Markdown
Contributor Author

@Ethan-Arrowood this is a WIP that i've had going for a month or so, first hand crafted, and then gaining momentum today with Claude. I wanted to get it in front of you and Austin as it's one of the biggest hurdles to people building useful things inside Studio from their browser. Adapting the nextjs examples into create-harper would be an additional win that we could start doing there too, but I'm learning.

@Ethan-Arrowood
Copy link
Copy Markdown
Member

Yes, I totally agree this plugin is important

@dawsontoth dawsontoth changed the title Production feat: Support SSR, SSG and HMR Jun 4, 2026
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.

2 participants