refactor: migrate ssrLoadModule to moduleRunner.import

[james-elicx]

Summary

• Replaces all server.ssrLoadModule() calls in dev-server.ts and api-handler.ts with runner.import() via the ModuleImporter interface, following the Vite Module Runner migration guide
• Removes ssrFixStacktrace / ssrRewriteStacktrace calls — these are not needed when using Module Runner APIs
• Updates call sites in index.ts to pass the existing getPagesRunner() lazy factory as the new runner argument

Files changed

File	Change
packages/vinext/src/server/dev-server.ts	All 18 ssrLoadModule calls → runner.import(), ssrFixStacktrace removed, runner: ModuleImporter added to createSSRHandler and renderErrorPage signatures
packages/vinext/src/server/api-handler.ts	server: ViteDevServer → runner: ModuleImporter, ssrLoadModule → runner.import(), ssrFixStacktrace removed
packages/vinext/src/index.ts	Updated two call sites to pass getPagesRunner() as the runner
tests/api-handler.test.ts	MockServer type + mockServer() helper replaced with ModuleImporter-shaped mock; removed stale ssrFixStacktrace assertion
tests/pages-router.test.ts	createSSRHandler call updated to pass a runner derived from the existing mock logic

Testing

• All 40 api-handler unit tests pass
• pnpm run check is clean (2 pre-existing benchmark errors unrelated to this change)

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@570

commit: 0ad4a4f

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original

[james-elicx]

/bigbonk review

[ask-bonk]

Review: Migrate ssrLoadModule to ModuleRunner

Clean, well-scoped refactor. The migration from server.ssrLoadModule() to runner.import() via the ModuleImporter interface is correct and consistent. All CI checks pass.

What's good

• Complete migration: All 18 ssrLoadModule calls in dev-server.ts and the call in api-handler.ts are replaced. No stale references remain (only comments explaining the old pattern).
• Interface reuse: ModuleImporter was already defined in instrumentation.ts and used by runInstrumentation and runMiddleware. This PR extends its use to the remaining call sites — consistent with the existing pattern.
• server param retained where needed: createSSRHandler and renderErrorPage still accept ViteDevServer because they need server.transformIndexHtml(). The runner is additive, not a replacement for server in those signatures.
• Test updates are correct: The api-handler.test.ts mock is now a clean ModuleImporter shape (no more as unknown as MockServer cast). The pages-router.test.ts mock extracts loadModule as a shared function used by both server.ssrLoadModule (still needed for the ViteDevServer cast) and runner.import.

Minor observations (non-blocking)

1. sourcemapInterceptor: false in createDirectRunner — The PR description says "ssrFixStacktrace/ssrRewriteStacktrace are not needed when using Module Runner APIs." This is technically accurate for ssrFixStacktrace specifically (it only applies to ssrLoadModule-loaded modules), but the direct runner has sourcemapInterceptor: false, which means the Module Runner is not doing automatic source map rewriting either. In dev, Node's --enable-source-maps or Vite's dev server error overlay handles this, so it's fine in practice. But the comments "Stack traces are automatically rewritten by the ModuleRunner" at dev-server.ts:1001 and api-handler.ts:245 are slightly misleading — the runner created by createDirectRunner has source map interception disabled. Consider updating the comments to say something like "ssrFixStacktrace is specific to ssrLoadModule and is not applicable when using ModuleRunner" instead.

2. ALS-ARCHITECTURE.md still references ssrLoadModule — Lines 81 and 95 still say server.ssrLoadModule("vinext/head-state"). These are architectural docs, not code, so it's low priority, but worth a follow-up update for accuracy.

3. eslint-disable comments — There are ~12 new eslint-disable-next-line @typescript-eslint/no-explicit-any comments for the as Record<string, any> casts on runner.import(). This is a consequence of ModuleImporter.import() returning Promise<unknown>. An alternative would be a typed helper like async function importModule(runner: ModuleImporter, id: string): Promise<Record<string, any>> to centralize the cast and eliminate the inline disables. Not blocking, but would reduce noise.

4. Stale mock in pages-router.test.ts — The test at line 2957 still sets ssrLoadModule: vi.fn(loadModule) on the mock server object. Since createSSRHandler no longer calls server.ssrLoadModule, this mock property is dead code. Removing it would make the test clearer about what's actually exercised.

Looks good overall.

[ask-bonk]

Review posted and approved on PR #570. Here's a summary:

Approved — This is a clean, well-scoped refactor that correctly migrates all ssrLoadModule() calls to runner.import() via the existing ModuleImporter interface.

I raised 4 non-blocking observations:

1. Slightly misleading comments — The comments at dev-server.ts:1001 and api-handler.ts:245 say "Stack traces are automatically rewritten by the ModuleRunner," but createDirectRunner has sourcemapInterceptor: false. The comments should clarify that ssrFixStacktrace is simply not applicable to ModuleRunner, rather than claiming automatic rewriting.

2. Stale doc references — ALS-ARCHITECTURE.md still mentions server.ssrLoadModule() in two places.

3. eslint-disable noise — 12 new inline eslint-disable comments for as Record<string, any> casts could be centralized with a typed helper function.

4. Dead mock in test — pages-router.test.ts:2957 still sets ssrLoadModule on the mock server even though createSSRHandler no longer calls it.

github run

[james-elicx]

/bigbonk re-review

[ask-bonk]

Review: Migrate ssrLoadModule to ModuleRunner

This is a clean, complete migration. All 18 ssrLoadModule calls in dev-server.ts and the call in api-handler.ts are replaced with runner.import() via the existing ModuleImporter interface. No functional call sites were missed.

What's correct

• Complete coverage: Every ssrLoadModule call in dev-server.ts and api-handler.ts is migrated. The remaining ssrLoadModule reference at tests/pages-router.test.ts:999 is in the "Virtual server entry generation" test, which tests loading the virtual module itself (not the SSR handler path) — correctly untouched.

• importModule helper: The new importModule() function in instrumentation.ts:58-63 centralises the as Record<string, any> cast, which directly addresses ask-bonk's observation #3 about eslint-disable noise. The remaining inline runner.import() calls at lines 259-260 (vinext/head-state, vinext/router-state), line 363, line 613, and the _app/_document loads are for registration-only or structural loads where the return value only needs .default — using the raw runner.import() with a one-off cast is fine there.

• server param retained correctly: createSSRHandler and renderErrorPage still accept ViteDevServer because they call server.transformIndexHtml() and read server.config.root. The runner is purely additive.

• No prod-server/worker-entry changes needed: I verified prod-server.ts and worker-entry.ts — they don't use ssrLoadModule (production uses bundled imports, not runtime module loading). The AGENTS.md parity check passes.

• ALS-ARCHITECTURE.md updated: Lines 81 and 95 now correctly reference runner.import() and the ModuleImporter interface instead of server.ssrLoadModule(). This addresses ask-bonk's observation #2.

One minor observation

The comments at dev-server.ts:990 and api-handler.ts:244 say "ssrFixStacktrace() is specific to ssrLoadModule and is not applicable when using ModuleRunner." This is accurate and better than what ask-bonk suggested the original comments said. The current wording correctly explains why the call was removed without making any claim about automatic rewriting. Good.

LGTM — approve.

[ask-bonk]

Review posted and approved on PR #570. Here's a summary of my findings:

Approved — this is a clean, complete migration of all ssrLoadModule() calls to runner.import() via the ModuleImporter interface.

Key points from my review:

1. Complete coverage: All 18 ssrLoadModule calls in dev-server.ts and the call in api-handler.ts are migrated. The only remaining ssrLoadModule reference in test code (pages-router.test.ts:999) is in the "Virtual server entry generation" test which tests a different code path — correctly untouched.

2. importModule helper: The new centralised helper in instrumentation.ts addresses the eslint-disable noise concern from the earlier review. The few remaining inline runner.import() calls are for registration-only imports where the return value pattern is different.

3. Dev/prod parity verified: prod-server.ts and worker-entry.ts don't use ssrLoadModule (production uses bundled imports), so no changes needed there.

4. ALS-ARCHITECTURE.md updated: The docs now correctly reference runner.import() and ModuleImporter.

5. Comment accuracy: The ssrFixStacktrace removal comments are accurately worded — they explain why the call was removed without making incorrect claims about automatic rewriting.

github run