feat: use modulerunner for ssr#3645
Conversation
The switch from ssrLoadModule to ModuleRunner removed stack trace source mapping. Re-add setErrorInterceptor call so errors in route handlers get source-mapped before the error overlay renders them, and ssrFixStacktrace in the outer catch for module loading errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| ) as FetchHandler; | ||
| mod.setErrorInterceptor?.((err: unknown) => { | ||
| if (err instanceof Error) { | ||
| server.ssrFixStacktrace(err); |
There was a problem hiding this comment.
If this works to fix it, then that's great, but in theory it shouldn't be needed 🤔
server.ssrFixStacktraceandserver.ssrRewriteStacktracedo not have to be called when using the Module Runner APIs. The stack traces will be updated unlesssourcemapInterceptoris set tofalse. — SSR Using ModuleRunner API
I assumed it was because sourcemapInterceptor was indeed false because Deno had nooped process.setSourceMapsEnabled, but fixing that didn't seem to fix it.
Relevant Vite code: https://github.com/vitejs/vite/blob/99897d27b44dd73307fa03e2f11f0baa1a1dc939/packages/vite/src/node/ssr/runtime/serverModuleRunner.ts#L58-L72
fibibot
left a comment
There was a problem hiding this comment.
Direction matches the rest of the env-API migration from #3634 — ssrLoadModule is the old shim for the SSR env.
One concern: if (!isRunnableDevEnvironment(server.environments.ssr)) return; exits the middleware without calling next() or writing a response — if the SSR env is ever non-runnable, the request hangs. For Fresh's default config this branch is unreachable, but it's a sharp edge; either fall through to next() or assert with a thrown error so failures are loud.
Holding approval until CI is green — pr-title is red because the workflow ran with the original title "Use ModuleRunner for SSR" before the lowercase rename. An empty push or rebase should re-trigger and clear it.
- nit: the hand-rolled
FetchHandlertypessetErrorInterceptor?optional, forcing?.at the call site.server_entry.tsalways exports it — making the field required would surface a real regression if that export ever changes.
Extracted from/stacked upon #3634.