Review: Production Prerender Pipeline

Thorough review of 30 changed files, ~2500 additions. Overall this is a well-structured PR — the architecture is sound, the separation between prerender.ts (rendering engine), run-prerender.ts (orchestration), and static-export.ts (legacy adapter) is clean, and the test coverage is solid. A few issues worth discussing:

1. Prerendered routes are unreachable from vinext start (design question)

run-prerender.ts:146-149 writes non-export prerendered files to dist/server/prerendered-routes/. The prod-server.ts App Router handler (startAppRouterServer) only checks dist/client/ for static files (line 657), then falls through to the RSC handler for everything else. The prerendered HTML/RSC files in dist/server/prerendered-routes/ are never consulted.

This means vinext build --prerender-all && vinext start will regenerate every page on request despite having prerendered output sitting on disk. The files are effectively dead for the Node.js prod server.

The PR description explains this is intentional for Cloudflare Workers (where not_found_handling: "none" means every request hits the worker, and future KV pre-population will use these files). But for the Node.js vinext start path, there's no mechanism to serve them.

Questions:

• Is --prerender-all intended to be Workers-only for now? If so, a note in the CLI help text would prevent confusion.
• If vinext start should benefit from prerendered output, startAppRouterServer needs a second tryServeStatic check against the prerendered-routes/ directory before falling through to the RSC handler. This would be a simple addition:

  const prerenderDir = path.join(resolvedOutDir, "server", "prerendered-routes");
  if (pathname !== "/" && tryServeStatic(req, res, prerenderDir, pathname, compress)) {
    return;
  }

  Though this raises ISR staleness questions — prerendered ISR pages would be served forever without revalidation. Probably fine to defer this, but worth calling out.

2. Double render for App Router RSC payload

prerender.ts:805-812 — Each App Router URL is rendered twice: once for HTML, once for the RSC payload (with RSC: 1 header). For sites with many routes this doubles build time.

A single render pass that tees the RSC stream (or extracts the payload from the HTML response's embedded <script> data) would halve the work. Understandable as a v1 approach, but worth a // TODO comment noting the optimization opportunity for large sites.

3. buildUrlFromParams doesn't URL-encode param values

prerender.ts:185-211 — If generateStaticParams returns params with special characters (spaces, slashes, ?, #), the generated URL will be malformed:

// { slug: "hello world" } → "/blog/hello world" (unencoded)
result.push(String(value));

Should be result.push(encodeURIComponent(String(value))) for required params, and each element of catch-all arrays should be encoded individually.

4. Progress bar total is wrong when a phase has zero renderable URLs

run-prerender.ts:166-173 — The totalUrls counter is accumulated from the first onProgress callback of each phase. If a phase (e.g., App Router) has zero renderable URLs, onProgress never fires, and that phase's contribution to the total is silently zero. This is correct but fragile — it works because the guard if (appTotal === 0) only fires once, but it means the total is only known after the first URL from each phase finishes rendering.

Consider computing totals before rendering starts (from the urlsToRender.length / pagesToRender.length values) and passing them as a return value from a planning phase, rather than relying on the first callback.

5. Minor: nextConfigOverride is a shallow merge

run-prerender.ts:121 — { ...loadedConfig, ...options.nextConfigOverride } only merges top-level keys. A nested override like { images: { dangerouslyAllowSVG: true } } would replace the entire images object. Fine for the current test usage (output: "export" is top-level), but worth a comment noting the limitation.

6. Test coverage gaps (non-blocking)

The 36 tests cover the important paths well. A few gaps worth noting for future work:

• buildReportRows + prerenderResult upgrade — report.ts:264-266 upgrades unknown→static when prerender confirms the route, but build-report.test.ts doesn't exercise this code path with a prerenderResult argument.
• Progress callback assertions — No test verifies onProgress fires with correct completed/total values.
• trailingSlash: true — Not tested. getOutputPath handles it (prerender.ts:220), but no test exercises it.
• Redirect from getStaticProps — The meta-refresh HTML emission (prerender.ts:483-495) has no test coverage.

7. Snapshot changes look correct

The entry-templates.test.ts.snap changes show two meaningful additions:

1. generateStaticParamsMap export — correctly maps route patterns to their generateStaticParams functions, enabling the prerender phase to enumerate dynamic route URLs from the production bundle.
2. ISR revalidation now preserves searchParams — Previously always used new URLSearchParams(). Now correctly passes url.searchParams, fixing ISR pages that depend on query parameters.

Both are correct and expected given the PR's goals.

8. Test fixtures are well-designed

The static-export fixture covers a good cross-section: App Router static + dynamic (with generateStaticParams), Pages Router static + dynamic (with getStaticPaths), cross-router linking, and metadata generation. The Next.js 15+ thenable params pattern is used correctly.

Summary

The core architecture is solid. The main actionable items are:

1. Clarify whether --prerender-all output is intended to be inert for vinext start (or add serving logic)
2. Encode param values in buildUrlFromParams
3. Comment the double-render as a known optimization opportunity

Everything else is minor or future work. Nice PR — the static-export.ts refactor from 673 deletions down to a thin adapter is particularly clean.

Review: Production Prerender Pipeline

I've done a thorough read-through of all 30 changed files. The architecture is well-structured — the three-layer separation (prerender engine → orchestrator → legacy adapter) is clean, and the static-export.ts refactor from 673 lines down to a thin 136-line adapter is excellent. The test coverage at 36 new tests is solid for the core paths.

A few things I'd flag:

1. Catch-all array params are not URL-encoded in buildUrlFromParams

packages/vinext/src/build/prerender.ts:196-197 — When a catch-all param (:slug+ / :slug*) has an array value, the elements are spread directly without encoding:

if (Array.isArray(value)) {
  result.push(...value);  // ← not encoded
}

While the single-value branch (line 199) and required-param branch (line 204) both use encodeURIComponent, the array spread does not. If generateStaticParams returns something like { slug: ["docs", "hello world"] }, you'd get /docs/hello world instead of /docs/hello%20world.

Suggested fix:

if (Array.isArray(value)) {
  result.push(...value.map(v => encodeURIComponent(String(v))));
}

2. vinext start does not serve prerendered routes (design clarification needed)

The PR moves non-export prerendered output to dist/server/prerendered-routes/ (line 146-149 of run-prerender.ts), and the PR description explains why this is correct for Workers. However, startAppRouterServer in prod-server.ts:657 only checks dist/client/ for static files before falling through to the RSC handler. The prerendered files at dist/server/prerendered-routes/ are never consulted.

This means vinext build --prerender-all && vinext start will re-render every page on request despite having prerendered output on disk. The --prerender-all help text already says "(future releases will serve these files in vinext start)", so I think this is intentional for now — but worth confirming. If so, the output from --prerender-all is effectively inert for the Node.js prod server path until a follow-up PR adds the serving logic.

3. Double RSC handler invocation for App Router — TODO is present, good

prerender.ts:805-812 renders each URL twice (HTML + RSC payload). I see the TODO comment was already added at line 806:

// TODO: Extract RSC payload from the first response instead of invoking the handler twice.

This is the right approach for v1. For large sites this will double build time, but that optimization can come later.

4. runWithConcurrency worker count when items is empty

prerender.ts:169:

const workers = Array.from({ length: Math.min(concurrency, items.length || 1) }, worker);

When items.length === 0, this creates 1 worker that immediately exits (the while loop guard catches it). The || 1 prevents Math.min(N, 0) = 0 workers which would cause Promise.all([]) to resolve instantly with an empty array — which is actually correct behavior. The || 1 creates an unnecessary worker when items is empty. Harmless but slightly misleading. Consider:

if (items.length === 0) return results;

as an early return before the worker creation.

5. NoOpCacheHandler save/restore pattern is clean

The pattern of saving getCacheHandler(), setting NoOpCacheHandler, and restoring in finally (prerender.ts:357-361, prerender.ts:599-601) is well-done. This correctly isolates prerender from polluting the ISR cache and handles sequential test calls properly.

6. static-export.ts adapter is appropriately thin

The refactored static-export.ts is now a pure adapter layer — staticExportPages and staticExportApp just call prerenderPages/prerenderApp with mode: "export" and convert the result. The toStaticExportResult converter is clean. The comment at line 68-71 about pageCount vs files.length is a nice proactive clarification.

7. Speculative static rendering via Cache-Control detection is clever

prerender.ts:795-800 — Using the Cache-Control: no-store response header to detect dynamic usage during speculative renders is a smart approach. It piggybacks on the existing runtime signal rather than needing a separate side-channel. The error handling (lines 840-847) correctly catches thrown errors and falls back to "skipped/dynamic" for speculative routes while propagating errors for non-speculative ones.

8. Hybrid Pages Router build in cli.ts is correctly sequenced

cli.ts:396-417 — After the App Router buildApp(), a separate SSR-only build for Pages Router is triggered when hasPagesDir(). Using configFile: false with disableAppRouter: true prevents the App Router pipeline from interfering. emptyOutDir: false correctly preserves the RSC artifacts. This is the right approach for hybrid projects.

9. Minor: resolveParentParams lookup could be O(n²) for deeply nested dynamics

prerender.ts:254-273 — For each dynamic segment, allRoutes.find() does a linear scan. With deeply nested dynamic routes (e.g., /[a]/[b]/[c]/[d]/page.tsx), this is O(segments × routes). Not a real concern for any practical app, but if this ever becomes hot, indexing routes by pattern into a Map would help.

10. Test helpers: buildAppFixture and buildPagesFixture are well-isolated

The new helpers in tests/helpers.ts:195-253 build to fresh temp directories with unique prefixes, symlink node_modules for resolution, and return the bundle path. This allows parallel test execution without output directory conflicts. The rscOutDir/ssrOutDir/clientOutDir options added to VinextOptions are the right way to make this work.

Summary

The core architecture is solid. The main actionable item is #1 (URL-encoding catch-all array params). Item #2 is a design question worth clarifying in docs/help text. Everything else is either already addressed (double render TODO), non-blocking, or future optimization work. Clean PR.

Review: Production Prerender Pipeline

I've done a thorough read of all key changed files (~3400 additions). The overall architecture is well-designed — the three-layer separation (prerender.ts engine → run-prerender.ts orchestrator → static-export.ts adapter) is clean, and the static-export.ts collapse is excellent. Focusing on issues the previous three reviews did not identify.

1. Race condition in runWithConcurrency — nextIndex is not atomically read-and-incremented

packages/vinext/src/build/prerender.ts:180-183

async function worker() {
  while (nextIndex < items.length) {
    const i = nextIndex++;
    results[i] = await fn(items[i], i);
  }
}

This is safe only because JavaScript is single-threaded and nextIndex++ happens synchronously before the await. The while check and the increment are in the same microtask, so no interleaving is possible. Worth a comment stating this relies on the single-threaded event loop — someone reviewing this later might add a synchronization mechanism unnecessarily, or refactor it into something that breaks the invariant.

(Non-blocking — correctness is fine, just a readability observation.)

2. generateStaticParamsMap only maps routes where isDynamic && pagePath — misses route groups with generateStaticParams on layouts

packages/vinext/src/entries/app-rsc-entry.ts:1457-1465

export const generateStaticParamsMap = {
  ${routes
    .filter((r) => r.isDynamic && r.pagePath)
    .map(r => `  ${JSON.stringify(r.pattern)}: ${getImportVar(r.pagePath!)}?.generateStaticParams ?? null,`)
    .join("\n")}
};

This extracts generateStaticParams from the page module only (r.pagePath). In Next.js, generateStaticParams can also be exported from a layout (layout.tsx) to provide params for all child routes. A layout with generateStaticParams would not be captured here since the map only looks at pagePath.

For the current scope this may be acceptable (pages exporting generateStaticParams covers the 95% case), but it's a Next.js parity gap worth noting as a TODO.

3. resolveParentParams finds parents by matching a prefix pattern against allRoutes — but only matches routes with a pagePath

packages/vinext/src/build/prerender.ts:278-289

const parentRoute = allRoutes.find((r) => r.pattern === prefixPattern);
if (parentRoute?.pagePath) {
  const fn = staticParamsMap[prefixPattern];
  ...
}

If a parent dynamic segment's generateStaticParams is on a layout (not a page), the parent won't be found by this lookup (since staticParamsMap only includes pagePath entries per point #2 above). This means nested dynamic routes where the parent provides params via a layout's generateStaticParams would fall back to an empty parentSegments array, and the child's generateStaticParams would be called with params: {} instead of receiving the parent's param values. This could silently produce incorrect URLs.

4. Proxy-based staticParamsMap for CF Workers always returns a function — typeof fn !== "function" check is bypassed

packages/vinext/src/build/prerender.ts:748-767

The Proxy's get trap always returns an async function, and the has trap always returns true:

staticParamsMap = new Proxy({} as typeof staticParamsMap, {
  get(_target, pattern: string) {
    return async ({ params }) => { ... };
  },
  has(_target, _pattern) { return true; },
});

In prerenderApp at line 844, there's a check:

const generateStaticParamsFn = staticParamsMap[route.pattern];
if (typeof generateStaticParamsFn !== "function") { ... skip ... }

With the Proxy, this check will never be false — every pattern will appear to have a generateStaticParams function. When the worker endpoint returns "null" (no generateStaticParams for that route), the function returns null, and then line 874 does:

if (!Array.isArray(paramSets) || paramSets.length === 0) {
  results.push({ route: route.pattern, status: "skipped", reason: "no-static-params" });
}

So the skip still happens, but it goes through an unnecessary HTTP round-trip to the worker for every dynamic route that doesn't have generateStaticParams. For a project with many dynamic routes, this could add non-trivial latency to the prerender phase. Consider having the Proxy's get return null when the worker responds with "null", and restructure the caller to handle this, or better yet, fetch the list of patterns that have generateStaticParams upfront.

5. VINEXT_PRERENDER env var is set on process.env — leaks across concurrent test runs

packages/vinext/src/build/prerender.ts:387,673

process.env.VINEXT_PRERENDER = "1";

If tests run prerender functions concurrently in the same Node process (which is possible since the test file uses beforeAll per describe block), a finally block in one test could delete process.env.VINEXT_PRERENDER while another concurrent prerender is still running. The tests currently avoid this by running sequentially (describe blocks with beforeAll), but this is fragile.

For CF Workers builds, the env var is passed as a vars binding (vars: { VINEXT_PRERENDER: "1" }), which is isolated per worker instance — that's correct. But the Node path mutates shared global state. Consider using an ALS-scoped flag instead, or at minimum add a comment noting the assumption of sequential execution.

6. classifyPagesRoute uses static file analysis for route classification — may diverge from runtime behavior

packages/vinext/src/build/prerender.ts:489

const { type, revalidate: classifiedRevalidate } = classifyPagesRoute(route.filePath);

For Pages Router, classifyPagesRoute does static analysis of the source file to determine if it exports getServerSideProps, getStaticProps, etc. But the prerender phase also has access to the built bundle (bundlePageRoutes), which contains route.module.getStaticProps and route.module.getServerSideProps as actual runtime functions.

For plain Node builds, the runtime module exports could be used for a more accurate classification (the actual bundle module is loaded at bundleExports.pageRoutes). The current approach works because static analysis and runtime behavior should agree, but if they ever diverge (e.g., conditional exports, barrel file re-exports that confuse the static analyzer), the prerender phase would misclassify routes.

Not a bug today, but worth considering whether to prefer runtime module inspection over static analysis when the bundle is available.

7. No test for onProgress callback behavior

The PrerenderProgressCallback type and the progress reporting in both prerenderPages and prerenderApp are tested indirectly (they don't crash), but no test asserts that onProgress fires with correct completed/total values. Given that run-prerender.ts:217-224 uses the callback to compute a combined progress bar, incorrect values could produce confusing UX (progress bar jumping backwards, percentages > 100%).

A minimal test would be:

const progressCalls: { completed: number; total: number }[] = [];
await prerenderApp({
  ...opts,
  onProgress: ({ completed, total }) => progressCalls.push({ completed, total }),
});
expect(progressCalls.at(-1)?.completed).toBe(progressCalls.at(-1)?.total);

8. wranglerEntry resolution uses a hardcoded internal path

packages/vinext/src/build/prerender.ts:711-712 and run-prerender.ts:181:

const wranglerEntry = path.resolve(projectRoot, "node_modules/wrangler/wrangler-dist/cli.js");

This assumes wrangler's internal file structure (wrangler-dist/cli.js). This is a fragile assumption — wrangler versions could restructure their dist output. The pattern appears in two places (once in prerender.ts, once in run-prerender.ts), violating DRY. Consider extracting a resolveWranglerModule() helper that falls back to require.resolve("wrangler") if the hardcoded path doesn't exist.

9. tryServeStatic .html fallback applies to all static directories equally — no ISR staleness control

packages/vinext/src/server/prod-server.ts:290-304

The new .html fallback logic (try <path>.html, then <path>/index.html) applies to dist/client/ which currently only contains build-time assets. This is fine today, but if prerendered files are ever moved to dist/client/ (e.g., for a future vinext start integration as noted in the PR's future work), ISR pages would be served forever without revalidation because tryServeStatic has no awareness of revalidation headers or vinext-prerender.json.

The PR description acknowledges this explicitly ("Keeping prerendered output out of dist/client/ also prevents a future issue where KV pre-population would cause ISR routes to be served as stale static files forever"). Just confirming this is correctly isolated — the .html fallback in tryServeStatic should remain ISR-unaware, and any future vinext start prerender serving needs its own cache-aware serving path. No action needed now.

10. Positive highlights

• The generateStaticParamsMap export from the RSC entry is an elegant way to enable prerendering without ViteDevServer. Loading from the production bundle means the prerender phase exercises the exact same code paths as production serving.

• The NoOpCacheHandler save/restore pattern in finally blocks is well-done. The global Symbol.for key ensures it works across module instances.

• The CF Workers hybrid build detection (wrangler.json alongside the bundle) and shared wranglerDev instance across prerender phases is a solid design choice — avoids double miniflare startup costs.

• The speculative static rendering approach (render → check Cache-Control: no-store) is pragmatic and avoids needing a separate analysis pass.

• Test isolation via buildAppFixture/buildPagesFixture with temp directories and symlinked node_modules enables parallel test execution correctly.

Summary

The architecture is solid and well-documented. The main actionable items are:

1. #4 (Proxy always returns a function): Unnecessary HTTP round-trips for non-generateStaticParams routes in CF builds
2. #2/#3 (Layout generateStaticParams): Next.js parity gap worth a TODO comment
3. #8 (Hardcoded wrangler path): Fragile and duplicated — extract a helper

Everything else is non-blocking or future work. Clean PR.

Review: Production Prerender Pipeline

Read all 47 changed files (~3600 additions, 907 deletions). The architecture is well-designed — the three-layer separation (prerender.ts engine → run-prerender.ts orchestrator → static-export.ts adapter) is clean, and the static-export.ts collapse from 673 lines to a 136-line adapter is a substantial improvement. The test coverage at 36 new tests is solid. Below are my findings, focusing on issues the previous reviews did not identify or where I disagree with prior assessments.

1. [Security] Internal prerender endpoints are exposed in production

packages/vinext/src/entries/app-rsc-entry.ts:1567-1580 and 1595-1615

The /__vinext/prerender/static-params and /__vinext/prerender/pages-static-paths endpoints are unconditionally handled in the generated RSC entry with no environment gate. The comment at line 1563-1566 explains why there's no process.env.VINEXT_PRERENDER check (CF Workers vars aren't in process.env), but the consequence is that these endpoints are live in every production deployment.

An attacker can call GET /__vinext/prerender/static-params?pattern=/blog/:slug on any deployed worker and enumerate every route's generateStaticParams output, including potentially sensitive path parameters. The parentParams query parameter is JSON.parse()-d and passed directly to generateStaticParams({ params }) — if any generateStaticParams implementation makes database queries or API calls based on params, this becomes an unauthenticated data enumeration vector.

Suggested fix: Gate these endpoints on the VINEXT_PRERENDER env var binding. For CF Workers, read it from the env argument (the second argument to the fetch handler). Something like:

// Only respond during prerender (VINEXT_PRERENDER is set as a wrangler var)
const isPrerenderMode = typeof env !== "undefined" && env.VINEXT_PRERENDER === "1";
if (pathname === "/__vinext/prerender/static-params" && isPrerenderMode) {
  ...
}

The env parameter is available in the handler signature — you already pass env through from the Worker entry. For local unstable_dev calls, the vars: { VINEXT_PRERENDER: "1" } option already sets this binding.

2. [Bug] buildUrlFromParams produces "undefined" for missing required params

packages/vinext/src/build/prerender.ts:237-240

When a required param (:slug) is missing from the params object, params[paramName] returns undefined, and encodeURIComponent(String(undefined)) produces the literal string "undefined":

} else if (part.startsWith(":")) {
  const paramName = part.slice(1);
  const value = params[paramName];
  result.push(encodeURIComponent(String(value))); // "undefined" if missing
}

This would generate URLs like /blog/undefined which would silently render a garbage page rather than erroring. If generateStaticParams returns { slug: undefined } (a realistic bug in user code), the prerender succeeds with a wrong URL.

Suggested fix: Add a guard:

const value = params[paramName];
if (value === undefined || value === null) {
  throw new Error(
    `Missing required param "${paramName}" for route "${pattern}". ` +
    `generateStaticParams must return a value for every dynamic segment.`
  );
}
result.push(encodeURIComponent(String(value)));

3. [Design] resolveParentParams only searches for page routes, not layout routes

packages/vinext/src/build/prerender.ts:297-307 and packages/vinext/src/entries/app-rsc-entry.ts:1457-1465

The generateStaticParamsMap only includes entries where r.isDynamic && r.pagePath, and resolveParentParams requires parentRoute?.pagePath to find parent dynamic segments. This means generateStaticParams exported from a layout is invisible to the prerender phase.

In Next.js, generateStaticParams on a layout provides params for all child routes. For example:

app/[team]/layout.tsx         ← exports generateStaticParams
app/[team]/settings/page.tsx  ← no generateStaticParams needed

The current implementation would skip /[team]/settings with "no-static-params" because it can't find the parent layout's generateStaticParams.

This is acknowledged in prior reviews as a parity gap. I'd suggest at minimum a TODO comment in generateStaticParamsMap and resolveParentParams noting this limitation, and potentially a follow-up issue.

4. [Correctness] classifyPagesRoute uses source file analysis while bundle is available

packages/vinext/src/build/prerender.ts:568

For Pages Router, route classification calls classifyPagesRoute(route.filePath) which reads the source file and does regex matching for getServerSideProps, getStaticProps, etc. But at this point in the prerender phase, the built bundle is already loaded and bundlePageRoutes[i].module contains the actual runtime exports (getStaticPaths, getStaticProps, getServerSideProps).

The static file analysis and runtime exports should agree in most cases, but they can diverge when:

• The page re-exports data fetching functions from a shared module (export { getServerSideProps } from '../lib/ssr')
• Barrel files confuse the regex
• The source uses conditional exports

For the Node path, you could use the module exports directly:

const hasSSP = typeof route.module.getServerSideProps === "function";
const hasGSP = typeof route.module.getStaticProps === "function";

This would be more accurate. For the CF Workers path (where the module isn't directly available), the static analysis fallback still makes sense.

5. [Robustness] loadBundle cache-bust uses mtime but doesn't handle rapid rebuilds

packages/vinext/src/build/prerender.ts:400-401

const mtime = fs.statSync(bundlePath).mtimeMs;
return (await import(`${pathToFileURL(bundlePath).href}?t=${mtime}`)) as Record<string, unknown>;

On fast filesystems (tmpfs, SSD), two builds within the same millisecond produce the same mtime, causing a stale cache hit. This is unlikely in production but could happen in tests where buildAppFixture is called in quick succession. A monotonic counter or random nonce would be safer:

const cacheBust = Date.now() + "_" + Math.random().toString(36).slice(2, 8);

6. [Non-blocking] Progress tracking relies on first callback

packages/vinext/src/build/run-prerender.ts:209-231

The combined progress bar total is accumulated from the first onProgress callback of each phase:

let appTotal = 0;
onProgress: ({ total, route }) => {
  if (appTotal === 0) {
    appTotal = total;
    totalUrls += total;
  }
  completedUrls += 1;
  ...

If either phase has zero renderable URLs, onProgress never fires and that phase's total stays 0. This is correct behavior (0 URLs = 0 progress) but fragile — a future refactor that calls onProgress before rendering starts (e.g., to report "starting phase") would break the appTotal === 0 guard.

Computing totals upfront (e.g., having prerenderApp/prerenderPages return a planned count before rendering) would be more robust.

7. [Testing] CF fixture builds to the fixture directory itself, not a tmpdir

tests/helpers.ts:269-297

buildCloudflareAppFixture returns root: fixtureDir and builds to fixtureDir/dist/. Unlike buildAppFixture which builds to a fresh tmpdir, the CF fixture writes into the source tree. The afterAll in the test cleans dist/ up, but if the test crashes or is killed, tests/fixtures/cf-app-basic/dist/ persists and could affect subsequent runs. This also means CF prerender tests cannot run in parallel with each other.

Consider building to a tmpdir and symlinking node_modules like the other fixture builders do.

8. [Non-blocking] nextConfigOverride shallow merge limitation

packages/vinext/src/build/run-prerender.ts:132-133

const config = options.nextConfigOverride
  ? { ...loadedConfig, ...options.nextConfigOverride }
  : loadedConfig;

This only merges top-level keys. A nested override like { images: { dangerouslyAllowSVG: true } } would replace the entire images object. Fine for the current test usage but worth a code comment.

Positive Highlights

• Three-layer architecture is well-conceived. The prerender engine (prerender.ts) handles rendering, the orchestrator (run-prerender.ts) handles scanning/progress/manifests, and static-export.ts is a pure adapter. Clean separation of concerns.

• Speculative static rendering via Cache-Control: no-store detection (prerender.ts:1101-1106) is a pragmatic approach that avoids a separate analysis pass. Piggybacks on existing runtime signals.

• NoOpCacheHandler save/restore (prerender.ts:455-456, 750-751) with finally blocks is well-done. The global Symbol.for key ensures it works across module instances.

• generateStaticParamsMap export from the RSC entry (app-rsc-entry.ts:1457-1465) is an elegant way to enable prerendering without ViteDevServer — loading from the production bundle means the prerender phase exercises the exact same code paths as production serving.

• Shared wrangler instance for hybrid CF builds (run-prerender.ts:190-200) avoids double miniflare startup costs.

• Test isolation via buildAppFixture/buildPagesFixture with temp directories and symlinked node_modules enables parallel test execution correctly.

• URL encoding for catch-all array params (line 232) is correctly handled — this was flagged in earlier reviews and fixed.

• Double RSC render has a clear TODO comment (line 1112) — correct trade-off for v1.

• The runWithConcurrency empty-array early return (line 203) from earlier review feedback is implemented.

Summary

The main actionable items in priority order:

1. #1 (Security): Gate /__vinext/prerender/* endpoints on VINEXT_PRERENDER env binding — these are currently exposed in every production deployment
2. #2 (Bug): Guard against undefined params in buildUrlFromParams
3. #3 (Parity): Add TODO for layout-level generateStaticParams support
4. #4 (Correctness): Consider using runtime module exports instead of static analysis for Pages Router classification when bundle is available

Items #5-#8 are non-blocking improvements. The architecture is solid and the test coverage is good. Nice PR.

Review: Production Prerender Pipeline

Thorough read of all 47 changed files (~3660 additions, 907 deletions). The architecture is well-designed and this is a substantial, high-quality contribution. The three-layer separation is clean: prerender.ts (rendering engine) → run-prerender.ts (orchestration/progress) → static-export.ts (legacy adapter). The static-export.ts collapse from ~673 lines to 136 is an impressive simplification, and the test coverage with 36 new tests against real production builds is solid.

Below are my findings, prioritized by impact.

Issues

1. parentParams JSON.parse on the prerender endpoint should validate shape

packages/vinext/src/entries/app-rsc-entry.ts:1583-1584 — The /__vinext/prerender/static-params endpoint parses parentParams from query string and passes it directly to generateStaticParams:

const parentParams = url.searchParams.get("parentParams");
const params = parentParams ? JSON.parse(parentParams) : {};
const result = await fn({ params });

The VINEXT_PRERENDER=1 gate (line 1575) is correctly present — this won't fire in normal deployments. But within the prerender context, JSON.parse can throw on malformed input and the try/catch at line 1582 will catch it. The real concern is that params is passed to user-authored generateStaticParams without any type validation. If the input is a non-object (e.g. parentParams=5), the user function receives { params: 5 } instead of { params: {} }.

Suggestion: Add a guard after parse:

const raw = parentParams ? JSON.parse(parentParams) : {};
const params = (typeof raw === "object" && raw !== null && !Array.isArray(raw)) ? raw : {};

Non-blocking since the endpoint is only reachable during prerender, but makes the contract explicit.

2. loadBundle cache-bust relies on mtime — fine for production, potentially flaky in tests

packages/vinext/src/build/prerender.ts:412-413:

const mtime = fs.statSync(bundlePath).mtimeMs;
return (await import(`${pathToFileURL(bundlePath).href}?t=${mtime}`)) as Record<string, unknown>;

On fast SSDs or tmpfs, two builds within the same millisecond can produce identical mtimes, giving a stale cache hit. The comment says "Same mtime = same content = cache hit" which is generally true for production, but in rapid test loops where buildAppFixture is called multiple times, the assumption could break. Node's ESM loader cache keys on the full URL including query string, so the mtime-based approach is sound when mtimes actually differ.

Worth noting as a test-brittleness risk. A monotonic counter (Date.now() + random) would be marginally safer but the current approach is reasonable.

3. CF Workers hybrid test builds to fixture source tree, not tmpdir

tests/helpers.ts:269-297 — buildCloudflareAppFixture returns root: fixtureDir and writes output to fixtureDir/dist/. Unlike buildAppFixture and buildPagesFixture which use fresh tmpdirs, the CF fixture writes directly into the git-tracked source tree. The afterAll in the test cleans up dist/, but if the test crashes or is killed, tests/fixtures/cf-app-basic/dist/ persists.

This also means CF prerender tests can't run in parallel. The existing test framework runs serially so this is safe today, but it's inconsistent with the other fixture builders' isolation pattern.

Suggestion: Add a .gitignore entry for tests/fixtures/cf-app-basic/dist/ to prevent accidental commits if cleanup fails.

4. resolveParentParams — O(routes) per segment, quadratic for deep nesting

packages/vinext/src/build/prerender.ts:303 does allRoutes.find() for each dynamic segment prefix. For /[a]/[b]/[c]/[d]/page.tsx, this is 3 prefix lookups × N routes = O(segments × routes). Not a real concern for any practical app today — the typical depth is 2-3 levels. The PR description already lists "Performance improvements for resolveParentParams lookups" as future work. Indexed lookup by pattern would be trivial when needed.

Design Observations (non-blocking)

5. nextConfigOverride shallow merge

packages/vinext/src/build/run-prerender.ts:132-133 — { ...loadedConfig, ...options.nextConfigOverride } only merges top-level keys. Nested overrides like { images: { dangerouslyAllowSVG: true } } would replace the entire images object. This is fine for the current test usage (top-level keys only), but a comment noting the limitation would help future maintainers.

6. Progress bar total tracking

packages/vinext/src/build/run-prerender.ts:209-231 — The combined progress total is accumulated from the first onProgress callback of each phase. If a phase has zero renderable URLs, onProgress never fires and that phase's total stays 0. The behavior is correct (0 URLs = 0 progress), but it means the total is unknown until after the first URL from each phase finishes. Computing totals upfront from urlsToRender.length / pagesToRender.length and returning them from a planning step would be more robust, but this is a cosmetic UX concern.

7. classifyPagesRoute vs runtime module exports

packages/vinext/src/build/prerender.ts:580 — For Node builds, the code now correctly checks route.module.getServerSideProps/getStaticProps at runtime (lines 586-593) and only falls back to classifyPagesRoute file analysis for CF builds. This is the right approach — previous reviews flagged this and it's been addressed. The effectiveType = runtimeType ?? type pattern is clean.

Positive Highlights

• Speculative static rendering via Cache-Control: no-store detection (prerender.ts:1126-1131) is a pragmatic design. It piggybacks on existing runtime signals (headers(), cookies(), noStore() all set this header) rather than requiring a separate analysis pass. Routes classified as unknown by static analysis can be speculatively confirmed as static without any new API surface.

• NoOpCacheHandler save/restore pattern (prerender.ts:467-468, 775-776) is well-done. Saving the handler before prerender and restoring in finally correctly isolates prerender from polluting the ISR cache while keeping the global Symbol.for key approach consistent with the existing cache architecture.

• The generateStaticParamsMap export from the RSC entry (app-rsc-entry.ts:1457-1470) is an elegant way to enable prerendering from production bundles without ViteDevServer. The prerender phase exercises the exact same code paths as production serving.

• Shared wrangler instance for CF hybrid builds (run-prerender.ts:190-200) avoids double miniflare startup costs. The try/finally cleanup with ownedWranglerDev vs caller-provided _wranglerDev distinction is correctly handled.

• Test infrastructure — buildAppFixture and buildPagesFixture build to fresh tmp directories with unique prefixes and symlinked node_modules. The rscOutDir/ssrOutDir/clientOutDir options added to VinextOptions are the right way to make parallel test execution work without output conflicts.

• Hybrid Pages Router build in cli.ts:397-455 — After buildApp(), the separate SSR build uses configFile: false with disableAppRouter: true and emptyOutDir: false. The plugin inheritance logic (filtering out vinext/rsc/cloudflare plugins and re-registering vinext with disableAppRouter) is correct and preserves user transform plugins.

• Internal prerender endpoints are properly gated on VINEXT_PRERENDER=1 (app-rsc-entry.ts:1575, 1608). The gating works for both Node (process.env) and CF Workers (wrangler vars exposed via process.env in bundled workers). This was a security concern raised in earlier reviews and is addressed.

• buildUrlFromParams correctly validates required params with a clear error message (prerender.ts:239-244) and URL-encodes all branches including catch-all arrays (prerender.ts:232). Both issues from earlier reviews are fixed.

• Double RSC render has a clear TODO at line 1137 — correct trade-off for v1.

• static-export.ts is now a pure adapter — 4 functions totaling 136 lines. The toStaticExportResult converter is clean, and the comment about pageCount vs files.length (lines 67-70) is a proactive clarification.

Summary

The core architecture is solid. The main actionable item is #1 (input validation on the prerender endpoint). Items #2-#4 are non-blocking observations. The rest of the PR is well-designed, well-documented, and well-tested. The future work items listed in the PR description are reasonable scope deferrals.

Clean PR — the separation of the prerender engine from the orchestration layer and the legacy adapter is particularly well done.

Review: Production Prerender Pipeline

I've read all 49 changed files (~3800 additions, 936 deletions) in detail. This is an excellent PR — the architecture is well-designed, the separation of concerns is clean, and the test coverage is solid. The static-export.ts collapse from 673 lines to a thin 136-line adapter is a particularly nice outcome.

Below are my findings, organized by severity.

Architecture Assessment

The three-layer design is sound:

• prerender.ts (engine) — route classification, rendering, file I/O
• run-prerender.ts (orchestrator) — route scanning, progress UI, manifest writing, shared wrangler lifecycle
• static-export.ts (adapter) — preserves legacy StaticExportResult API

Key design decisions are well-motivated and clearly documented:

• Output to dist/server/prerendered-routes/ (not dist/client/) prevents ISR staleness with future KV pre-population
• NoOpCacheHandler save/restore pattern correctly isolates prerender from ISR cache pollution
• Speculative static rendering via Cache-Control: no-store detection piggybacks on existing runtime signals
• Shared wrangler instance for hybrid CF builds avoids double miniflare startup cost
• generateStaticParamsMap export from RSC entry enables prerendering from production bundles without ViteDevServer

Issues

1. completed counter in onProgress is not concurrency-safe (non-blocking, cosmetic)

prerender.ts:653,699 (Pages) and prerender.ts:1181,1185 (App) — The completed and completedApp counters use ++completed inside runWithConcurrency workers. Because JavaScript is single-threaded and the increment happens synchronously before the next await, this is correct — no race is possible. However, it's subtle enough that a comment noting the single-threaded safety guarantee would prevent a future maintainer from adding unnecessary synchronization or breaking the invariant during refactoring.

2. classifyPagesRoute revalidate value is used even when runtime type overrides static analysis

prerender.ts:580-613 — When runtimeType overrides the static analysis type (e.g., runtime says "ssg" but static analysis said "static"), the revalidate value still comes from classifiedRevalidate (the static analysis result). For most cases these agree, but if static analysis fails to detect a revalidate value that the runtime module actually sets (e.g., via a re-exported getStaticProps that sets revalidate: 60), the prerendered route would be tagged with revalidate: false instead of revalidate: 60.

This is a known limitation of the static analysis approach — and the classifyPagesRoute revalidate detection is already best-effort. But it's worth noting: the runtime module (route.module) is loaded and available at this point, so a future improvement could call getStaticProps({}) (with empty context) to extract the actual revalidate value, similar to how Next.js does it during its prerender phase.

3. Redirect meta-refresh HTML doesn't include a response body link

prerender.ts:665-686 — When getStaticProps returns a redirect, the generated HTML uses a <meta http-equiv="refresh"> tag. The <body> is empty. Best practice for meta-refresh redirects is to include a visible link in the body for clients that don't support meta-refresh (screen readers, some bots):

<body><p>Redirecting to <a href="${escapedDest}">${escapedDest}</a>...</p></body>

Non-blocking — the meta-refresh works for all modern browsers — but improves accessibility.

4. tryServeStatic HTML fallback serves prerendered pages with generic max-age=3600

prod-server.ts:290-308 — When the .html fallback matches a prerendered file, it's served with Cache-Control: public, max-age=3600 (the non-hashed default). For ISR pages with a specific revalidate interval, this cache header may not match the intended revalidation semantics. Currently this only affects dist/client/ (export mode), not dist/server/prerendered-routes/, so it's not an active problem. But when vinext start gains prerender serving, the cache headers should respect the route's revalidate value from vinext-prerender.json.

Design Observations (non-blocking)

5. Hybrid CF build: Pages Router routes are rendered through wrangler but also through the App Router worker

In run-prerender.ts:249-273, the Pages Router phase for CF builds uses sharedWranglerDev (the same worker that handles App Router). This means Pages Router getStaticPaths is fetched via the /__vinext/prerender/pages-static-paths endpoint on the App Router worker, and page rendering goes through the worker's fetch handler. This is the right approach for CF builds — there's no standalone Pages Router bundle for Workers. The code and comments make this clear; just confirming the design is correct.

6. runPrerender progress total tracking

run-prerender.ts:213,229-231 — The total is accumulated from the first onProgress callback of each phase. The comment at line 145-147 explains this. If a phase has zero renderable URLs, its total stays 0 (correct). The approach is fragile if onProgress semantics change, but is adequate for v1. Computing totals upfront from a planning phase would be more robust.

7. The wrangler type import uses Unstable_DevWorker

prerender.ts:23 — The name Unstable_DevWorker suggests this API may change. This is a reasonable choice given it's the only programmatic way to run a local miniflare instance, and vinext already depends on wrangler for deploy. Just noting the dependency on an unstable API.

Positive Highlights

• buildUrlFromParams validation (prerender.ts:239-244) — Required param validation with a clear error message prevents silent /blog/undefined generation. The catch-all array branch correctly encodes each element individually.

• VINEXT_PRERENDER gating on internal endpoints (app-rsc-entry.ts:1575,1612) — Both prerender endpoints check process.env.VINEXT_PRERENDER !== "1" and return 404 otherwise. For CF Workers, the var is injected via wrangler.unstable_dev({ vars: { VINEXT_PRERENDER: "1" } }), which miniflare exposes via process.env in bundled workers. This correctly prevents exposure in production.

• parentParams input validation (app-rsc-entry.ts:1584-1588) — Rejects non-object values (primitives, arrays, null) so user generateStaticParams always receives a plain object.

• Error handling throughout — Bundle-not-found errors direct users to run vinext build. Wrangler-not-installed errors suggest npm install --save-dev wrangler. Export mode errors name the specific route and explain what to fix. finally blocks consistently restore global state (cache handler, env vars, wrangler instances).

• Test isolation — buildAppFixture/buildPagesFixture use fresh tmpdirs with symlinked node_modules. buildCloudflareAppFixture uses createIsolatedFixture to copy the fixture before building. This enables parallel test execution without conflicts.

• Clean static-export.ts adapter — 4 functions totaling 136 lines. toStaticExportResult() correctly maps the new discriminated union to the legacy shape with a clear comment about pageCount vs files.length.

• Well-documented TODO comments — Layout-level generateStaticParams (prerender.ts:304-309, app-rsc-entry.ts:1458-1462), double RSC render optimization (prerender.ts:1137), and the resolveParentParams performance note in the PR description.

Summary

The architecture is solid, the code is well-documented, and the test coverage is comprehensive. The main items I'd call attention to are:

1. #2 (revalidate from static analysis vs runtime) — minor correctness gap, future improvement opportunity
2. #3 (meta-refresh accessibility) — non-blocking improvement
3. #4 (cache headers for prerendered pages) — note for future vinext start integration

Everything else is either already addressed or clearly documented as future work. Clean PR.