Skip to content

feat(prerender): usability enhancements for pre-rendering (#565) #732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
raed04 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat(prerender): usability enhancements for pre-rendering (#565) #732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 120 additions & 22 deletions packages/vinext/src/build/report.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
* execution). Vite's parseAst() is NOT used because it doesn't handle
* TypeScript syntax.
*
* Limitation: without running the build, we cannot detect dynamic API usage
* (headers(), cookies(), connection(), etc.) that implicitly forces a route
* dynamic. Routes without explicit `export const dynamic` or
* `export const revalidate` are classified as "unknown" rather than "static"
* to avoid false confidence.
* Dynamic API imports (headers(), cookies(), connection(), unstable_noStore())
* are detected heuristically and classified as "ssr". Routes without explicit
* config or detectable dynamic API usage are classified as "unknown" rather
* than "static" to avoid false confidence.
*/

import fs from "node:fs";
Expand All @@ -40,6 +39,10 @@ export type RouteRow = {
* Used by `formatBuildReport` to add a note in the legend.
*/
prerendered?: boolean;
/** Pre-render status from the prerender phase, if available. */
prerenderStatus?: "rendered" | "skipped" | "error";
/** For dynamic routes: the concrete URLs that were pre-rendered. */
prerenderPaths?: string[];
};

// ─── Regex-based export detection ────────────────────────────────────────────
Expand Down Expand Up @@ -98,6 +101,47 @@ export function extractExportConstNumber(code: string, name: string): number | n
return m[1] === "Infinity" ? Infinity : parseFloat(m[1]);
}

// ─── Dynamic API detection (module-level compiled regexes) ───────────────────

// Anchored to line start (^) to avoid matching commented-out imports.
// Uses multiline flag so ^ matches each line in multi-line source.
// Negative lookbehind (?<!\btype\s) skips inline type modifiers like
Comment on lines +106 to +108
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot Apr 10, 2026

Choose a reason for hiding this comment

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

The comment says the ^ anchor avoids matching commented-out imports, but this only works for single-line // comments. As confirmed by testing, a multi-line block comment where import starts on its own line is a false positive:

const x = 1; /*
import { headers } from "next/headers";
*/

This is acceptable for a heuristic, but the comment should document the limitation explicitly so future readers know:

Suggested change
// Anchored to line start (^) to avoid matching commented-out imports.
// Uses multiline flag so ^ matches each line in multi-line source.
// Negative lookbehind (?<!\btype\s) skips inline type modifiers like
// Anchored to line start (^) to avoid matching commented-out imports.
// Uses multiline flag so ^ matches each line in multi-line source.
// Limitation: multi-line block comments where `import` starts on a new line
// will produce false positives — this is a known tradeoff of regex-based analysis.
// Negative lookbehind (?<!\btype\s) skips inline type modifiers like

// `import { type cookies }` which are erased at compile time.

/** Detects `import { headers | cookies | draftMode } from "next/headers"` */
const DYNAMIC_HEADERS_RE =
/^\s*import\s*\{[^}]*(?<!\btype\s)\b(?:headers|cookies|draftMode)\b[^}]*\}\s*from\s*['"]next\/headers['"]/m;
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot Apr 1, 2026

Choose a reason for hiding this comment

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

The ^ anchor with /m correctly skips // import { headers } ... (single-line comments) because the // precedes import. However, it produces false positives for imports inside multi-line block comments that start on a prior line:

const x = 1; /*
import { headers } from "next/headers";
*/

Here import starts at the beginning of a line (matching ^ in multiline mode), but the code is dead.

This is an inherent limitation of regex-based analysis and the JSDoc already documents the heuristic nature, so it's fine as-is for a build report. But worth adding to the "Limitations" comment block so future readers know.


/** Detects `import { connection } from "next/server"` (stabilized replacement for unstable_noStore) */
const DYNAMIC_SERVER_RE =
/^\s*import\s*\{[^}]*(?<!\btype\s)\bconnection\b[^}]*\}\s*from\s*['"]next\/server['"]/m;

/** Detects `import { unstable_noStore | noStore } from "next/cache"` */
const DYNAMIC_CACHE_RE =
/^\s*import\s*\{[^}]*(?<!\btype\s)\b(?:unstable_noStore|noStore)\b[^}]*\}\s*from\s*['"]next\/cache['"]/m;

/**
* Detects imports of Next.js dynamic APIs that implicitly force a route dynamic.
* Checks for:
* - import { headers, cookies, draftMode } from "next/headers"
* - import { connection } from "next/server"
* - import { unstable_noStore, noStore } from "next/cache"
*
* This is a heuristic — importing a function doesn't guarantee it's called
* at render time (it could be conditionally used or in a non-render helper).
* But it provides better signal than "unknown" for the build report.
*
* Limitations:
* - Does not detect `require()` or dynamic `import()` calls
* - Does not detect `type` imports (which are erased at runtime — correct behavior)
* - Anchored to line start to avoid false positives from commented-out imports
*/
export function detectsDynamicApiUsage(code: string): boolean {
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot Apr 1, 2026

Choose a reason for hiding this comment

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

Naming nit: detectsDynamicApiUsage reads like a property/descriptor ("this function detects...") rather than an imperative. The rest of the codebase uses imperative names (extractExportConstString, hasNamedExport, classifyAppRoute). Consider detectDynamicApiUsage or hasDynamicApiImport for consistency.

Not a blocker since this is exported and already referenced in tests, but worth considering if you're touching it again.

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot Apr 10, 2026

Choose a reason for hiding this comment

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

Agreeing with the previous review: detectsDynamicApiUsage reads as a descriptor ("this function detects...") rather than an imperative. Every other function in this file uses imperative naming: extractExportConstString, hasNamedExport, classifyAppRoute.

Suggested change
export function detectsDynamicApiUsage(code: string): boolean {
export function detectDynamicApiUsage(code: string): boolean {

This is exported and referenced in tests, so the rename touches tests/build-report.test.ts too. Worth doing now before external consumers depend on the name.

return (
DYNAMIC_HEADERS_RE.test(code) || DYNAMIC_SERVER_RE.test(code) || DYNAMIC_CACHE_RE.test(code)
);
}

/**
* Extracts the `revalidate` value from inside a `getStaticProps` return object.
* Looks for: revalidate: <number> or revalidate: false or revalidate: Infinity
Expand Down Expand Up @@ -701,9 +745,15 @@ export function classifyAppRoute(
// Fall back to isDynamic flag (dynamic URL segments without explicit config)
if (isDynamic) return { type: "ssr" };

// No explicit config and no dynamic URL segments — we can't confirm static
// without running the build (dynamic API calls like headers() are invisible
// to static analysis). Report as unknown rather than falsely claiming static.
// Check for imports of dynamic APIs — these strongly suggest the route is
// dynamic even without explicit `export const dynamic` configuration.
// This improves on "unknown" but remains a heuristic: the import could be
// conditional or unused at render time.
if (detectsDynamicApiUsage(code)) return { type: "ssr" };

// No explicit config, no dynamic URL segments, and no detected dynamic API
// imports — we can't confirm static without running the build.
// Report as unknown rather than falsely claiming static.
return { type: "unknown" };
}

Expand All @@ -726,17 +776,33 @@ export function buildReportRows(options: {
}): RouteRow[] {
const rows: RouteRow[] = [];

// Build a set of routes that were confirmed rendered by speculative prerender.
const renderedRoutes = new Set<string>();
// Build maps from prerender results for route enrichment.
const prerenderStatusMap = new Map<string, "rendered" | "skipped" | "error">();
const prerenderPathsMap = new Map<string, string[]>();
if (options.prerenderResult) {
for (const r of options.prerenderResult.routes) {
if (r.status === "rendered") renderedRoutes.add(r.route);
// For rendered routes with a concrete path (dynamic routes expanded to URLs),
// collect all paths under the route pattern.
if (r.status === "rendered") {
prerenderStatusMap.set(r.route, "rendered");
if (r.path) {
const paths = prerenderPathsMap.get(r.route) ?? [];
paths.push(r.path);
prerenderPathsMap.set(r.route, paths);
}
} else if (!prerenderStatusMap.has(r.route)) {
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot Apr 1, 2026

Choose a reason for hiding this comment

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

The priority logic — "rendered" wins over "skipped"/"error" for the same route — is smart for dynamic routes with multiple paths where some succeed and some fail. But the else if (!prerenderStatusMap.has(r.route)) guard means the first non-rendered status wins among skipped/error. If a route has both skipped and error results, whichever appears first in the array determines the status.

Is that intentional? It seems unlikely in practice (a route is either skipped or errored, not both), but if it can happen, you might want error to take priority over skipped.

// Only set skipped/error if not already rendered (a dynamic route may
// have some rendered paths and some errors).
prerenderStatusMap.set(r.route, r.status);
}
Comment on lines +793 to +797
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot Apr 10, 2026

Choose a reason for hiding this comment

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

The status priority logic is: rendered always wins (overwrites any prior status), then first-seen non-rendered status sticks. This means if the results array has [{ route: "/x", status: "skipped" }, { route: "/x", status: "error" }], the route gets skipped — the error is silently hidden.

In practice a single route probably can't be both skipped and errored, but the defensive option would be to give error priority over skipped:

Suggested change
} else if (!prerenderStatusMap.has(r.route)) {
// Only set skipped/error if not already rendered (a dynamic route may
// have some rendered paths and some errors).
prerenderStatusMap.set(r.route, r.status);
}
} else if (!prerenderStatusMap.has(r.route)) {
prerenderStatusMap.set(r.route, r.status);
} else if (r.status === "error" && prerenderStatusMap.get(r.route) === "skipped") {
// Error takes priority over skipped for the same route.
prerenderStatusMap.set(r.route, r.status);
}

}
}

for (const route of options.pageRoutes ?? []) {
const { type, revalidate } = classifyPagesRoute(route.filePath);
rows.push({ pattern: route.pattern, type, revalidate });
const prerenderStatus = prerenderStatusMap.get(route.pattern);
const prerenderPaths = prerenderPathsMap.get(route.pattern);
rows.push({ pattern: route.pattern, type, revalidate, prerenderStatus, prerenderPaths });
}

for (const route of options.apiRoutes ?? []) {
Expand All @@ -745,11 +811,19 @@ export function buildReportRows(options: {

for (const route of options.appRoutes ?? []) {
const { type, revalidate } = classifyAppRoute(route.pagePath, route.routePath, route.isDynamic);
if (type === "unknown" && renderedRoutes.has(route.pattern)) {
const prerenderStatus = prerenderStatusMap.get(route.pattern);
const prerenderPaths = prerenderPathsMap.get(route.pattern);
if (type === "unknown" && prerenderStatus === "rendered") {
// Speculative prerender confirmed this route is static.
rows.push({ pattern: route.pattern, type: "static", prerendered: true });
rows.push({
pattern: route.pattern,
type: "static",
prerendered: true,
prerenderStatus,
prerenderPaths,
});
} else {
rows.push({ pattern: route.pattern, type, revalidate });
rows.push({ pattern: route.pattern, type, revalidate, prerenderStatus, prerenderPaths });
}
}

Expand All @@ -761,7 +835,7 @@ export function buildReportRows(options: {

// ─── Formatting ───────────────────────────────────────────────────────────────

const SYMBOLS: Record<RouteType, string> = {
export const SYMBOLS: Record<RouteType, string> = {
static: "○",
isr: "◐",
ssr: "ƒ",
Expand Down Expand Up @@ -804,8 +878,20 @@ export function formatBuildReport(rows: RouteRow[], routerLabel = "app"): string
const sym = SYMBOLS[row.type];
const suffix =
row.type === "isr" && row.revalidate !== undefined ? ` (${row.revalidate}s)` : "";
// Prerender annotation
let prerenderSuffix = "";
if (row.prerenderStatus === "rendered") {
const pathCount = row.prerenderPaths?.length;
prerenderSuffix = pathCount
? ` [prerendered: ${pathCount} path${pathCount !== 1 ? "s" : ""}]`
: " [prerendered]";
} else if (row.prerenderStatus === "skipped") {
prerenderSuffix = " [skipped]";
} else if (row.prerenderStatus === "error") {
prerenderSuffix = " [error]";
}
const padding = " ".repeat(maxPatternLen - row.pattern.length);
lines.push(` ${corner} ${sym} ${row.pattern}${padding}${suffix}`);
lines.push(` ${corner} ${sym} ${row.pattern}${padding}${suffix}${prerenderSuffix}`);
});

lines.push("");
Expand All @@ -819,11 +905,9 @@ export function formatBuildReport(rows: RouteRow[], routerLabel = "app"): string
// Explanatory note — only shown when unknown routes are present
if (usedTypes.includes("unknown")) {
lines.push("");
lines.push(" ? Some routes could not be classified. vinext currently uses static analysis");
lines.push(
" and cannot detect dynamic API usage (headers(), cookies(), etc.) at build time.",
);
lines.push(" Automatic classification will be improved in a future release.");
lines.push(" ? Some routes could not be fully classified by static analysis. Routes that");
lines.push(" import dynamic APIs (headers(), cookies(), etc.) are detected as dynamic,");
lines.push(" but other dynamic patterns may not be caught without running the build.");
}

// Speculative-render note — shown when any routes were confirmed static by prerender
Expand All @@ -836,6 +920,20 @@ export function formatBuildReport(rows: RouteRow[], routerLabel = "app"): string
lines.push(" succeeded without dynamic API usage).");
}

// Prerender summary — shown when any routes have prerender status
const hasAnyPrerender = rows.some((r) => r.prerenderStatus);
if (hasAnyPrerender) {
const renderedCount = rows.filter((r) => r.prerenderStatus === "rendered").length;
const skippedCount = rows.filter((r) => r.prerenderStatus === "skipped").length;
const errorCount = rows.filter((r) => r.prerenderStatus === "error").length;
lines.push("");
const summaryParts: string[] = [];
if (renderedCount > 0) summaryParts.push(`${renderedCount} prerendered`);
if (skippedCount > 0) summaryParts.push(`${skippedCount} skipped`);
if (errorCount > 0) summaryParts.push(`${errorCount} failed`);
lines.push(` Prerender: ${summaryParts.join(", ")}`);
}

return lines.join("\n");
}

Expand Down
Loading
Loading