Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
132 changes: 132 additions & 0 deletions packages/vinext/src/build/nitro-route-rules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import { appRouter, type AppRoute } from "../routing/app-router.js";
import { apiRouter, pagesRouter, type Route } from "../routing/pages-router.js";
import { buildReportRows, type RouteRow } from "./report.js";

// Mirrors Nitro's NitroRouteConfig — hand-rolled because nitropack is not a direct dependency.
export type NitroRouteRuleConfig = Record<string, unknown> & {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: consider importing this type from Nitro directly (e.g., import type { NitroRouteRules } from 'nitropack') if Nitro is a dependency, rather than hand-rolling the type. This ensures the type stays in sync with Nitro's actual API. If Nitro isn't a dependency (and shouldn't be), the hand-rolled type is fine — but add a comment noting it mirrors Nitro's NitroRouteConfig.

swr?: boolean | number;
cache?: unknown;
static?: boolean;
isr?: boolean | number;
prerender?: boolean;
};

export type NitroRouteRules = Record<string, { swr: number }>;

/**
* Scans the filesystem for route files and generates Nitro `routeRules` for ISR routes.
*
* Note: this duplicates the filesystem scanning that `printBuildReport` also performs.
* The `nitro.setup` hook runs during Nitro initialization (before the build), while
* `printBuildReport` runs after the build, so sharing results is non-trivial. This is
* a future optimization target.
*
* Unlike `printBuildReport`, this path does not receive `prerenderResult`, so routes
* classified as `unknown` by static analysis (which `printBuildReport` might upgrade
* to `static` via speculative prerender) are skipped here.
*/
export async function collectNitroRouteRules(options: {
appDir?: string | null;
pagesDir?: string | null;
pageExtensions: string[];
}): Promise<NitroRouteRules> {
const { appDir, pageExtensions, pagesDir } = options;

let appRoutes: AppRoute[] = [];
let pageRoutes: Route[] = [];
let apiRoutes: Route[] = [];

if (appDir) {
appRoutes = await appRouter(appDir, pageExtensions);
}

if (pagesDir) {
const [pages, apis] = await Promise.all([
pagesRouter(pagesDir, pageExtensions),
apiRouter(pagesDir, pageExtensions),
]);
pageRoutes = pages;
apiRoutes = apis;
}

return generateNitroRouteRules(buildReportRows({ appRoutes, pageRoutes, apiRoutes }));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: this re-scans the filesystem and re-reads every route file to extract revalidate — the same work printBuildReport does in cli.ts:513. For the initial implementation this is fine, but a follow-up optimization could cache route classification results from one pass and share them.

Also, printBuildReport supports a prerenderResult option that upgrades unknown routes to static when confirmed by speculative prerender. This path doesn't receive prerender results, so the route classification may differ slightly (some routes that printBuildReport would mark as static might appear as unknown here and be skipped). Worth documenting.

}

export function generateNitroRouteRules(rows: RouteRow[]): NitroRouteRules {
const rules: NitroRouteRules = {};

for (const row of rows) {
if (
row.type === "isr" &&
typeof row.revalidate === "number" &&
Number.isFinite(row.revalidate) &&
row.revalidate > 0
) {
rules[convertToNitroPattern(row.pattern)] = { swr: row.revalidate };
}
}

return rules;
}

/**
* Converts vinext's internal `:param` route syntax to Nitro's rou3
* pattern format. Nitro uses `rou3` for routeRules matching, which
* supports `*` (single-segment) and `**` (multi-segment) wildcards.
*
* /blog/:slug -> /blog/* (single segment)
* /docs/:slug+ -> /docs/** (one or more segments — catch-all)
* /docs/:slug* -> /docs/** (zero or more segments — optional catch-all)
* /about -> /about (unchanged)
* /:a/:b produces `/*`/`/*` (consecutive single-segment params)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line is garbled — the backtick-escaped /* fragments render as broken text. Since this is a code comment (not rendered markdown), backticks aren't needed and make it harder to read.

Suggested change
* /:a/:b produces `/*`/`/*` (consecutive single-segment params)
* /:a/:b -> /*/* (consecutive single-segment params)

*/
export function convertToNitroPattern(pattern: string): string {
return pattern
.split("/")
.map((segment) => {
if (segment.startsWith(":")) {
// Catch-all (:param+) and optional catch-all (:param*) match multiple segments → **
// Single dynamic param (:param) matches one segment → *
return segment.endsWith("+") || segment.endsWith("*") ? "**" : "*";
}
return segment;
})
.join("/");
Comment on lines +83 to +94
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:param (no suffix) matches exactly one URL segment in vinext's routing, but ** in rou3 matches zero or more segments. The correct single-segment wildcard in rou3 is *.

This means /blog/:slug/blog/** would apply the SWR rule to /blog/a/b/c (which only the route /blog/:slug+ should match). For ISR caching this is low-severity (those paths would 404 at the app level), but it's semantically incorrect and could cause confusion.

The fix is to distinguish between single-segment params and catch-all params:

Suggested change
export function convertToNitroPattern(pattern: string): string {
return pattern
.split("/")
.map((segment) => {
if (segment.startsWith(":")) {
return "**";
}
return segment;
})
.join("/");
export function convertToNitroPattern(pattern: string): string {
return pattern
.split("/")
.map((segment) => {
if (segment.startsWith(":")) {
// Catch-all (:param+) and optional catch-all (:param*) match multiple segments → **
// Single dynamic param (:param) matches one segment → *
return segment.endsWith("+") || segment.endsWith("*") ? "**" : "*";
}
return segment;
})
.join("/");
}

And update the tests accordingly:

  • /blog/:slug/blog/* (not /blog/**)
  • /users/:id/posts/users/*/posts (not /users/**/posts)
  • /docs/:slug+/docs/** (unchanged)
  • /docs/:slug*/docs/** (unchanged)
  • /:a/:b/*/** or /*/* depending on the param suffixes

}

export function mergeNitroRouteRules(
existingRouteRules: Record<string, NitroRouteRuleConfig> | undefined,
generatedRouteRules: NitroRouteRules,
): {
routeRules: Record<string, NitroRouteRuleConfig>;
skippedRoutes: string[];
} {
const routeRules = { ...existingRouteRules };
const skippedRoutes: string[] = [];

for (const [route, generatedRule] of Object.entries(generatedRouteRules)) {
const existingRule = routeRules[route];

if (existingRule && hasUserDefinedCacheRule(existingRule)) {
skippedRoutes.push(route);
continue;
}

routeRules[route] = {
...existingRule,
...generatedRule,
};
}

return { routeRules, skippedRoutes };
}

function hasUserDefinedCacheRule(rule: NitroRouteRuleConfig): boolean {
return (
rule.swr !== undefined ||
rule.cache !== undefined ||
rule.static !== undefined ||
rule.isr !== undefined ||
rule.prerender !== undefined
);
}
45 changes: 45 additions & 0 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import { generateServerEntry as _generateServerEntry } from "./entries/pages-server-entry.js";
import { generateClientEntry as _generateClientEntry } from "./entries/pages-client-entry.js";
import { appRouter, invalidateAppRouteCache } from "./routing/app-router.js";
import type { NitroRouteRuleConfig } from "./build/nitro-route-rules.js";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good — import type is erased at runtime, so this doesn't pull in build/nitro-route-rules.js during dev. The prior review comment about "top-level import bloat" was incorrect.

import { createValidFileMatcher } from "./routing/file-matcher.js";
import { createSSRHandler } from "./server/dev-server.js";
import { handleApiRoute } from "./server/api-handler.js";
Expand Down Expand Up @@ -1168,6 +1169,16 @@ export interface VinextOptions {
};
}

interface NitroSetupContext {
options: {
dev?: boolean;
routeRules?: Record<string, NitroRouteRuleConfig>;
};
logger?: {
warn?: (message: string) => void;
};
}

export default function vinext(options: VinextOptions = {}): PluginOption[] {
const viteMajorVersion = getViteMajorVersion();
let root: string;
Expand Down Expand Up @@ -4369,6 +4380,40 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
},
};
})(),
{
name: "vinext:nitro-route-rules",
nitro: {
setup: async (nitro: NitroSetupContext) => {
if (nitro.options.dev) return;
if (!nextConfig) return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: nextConfig is guarded here, but the nitro.setup hook fires during Nitro initialization, which happens during Vite's config() resolution phase. Since the vinext:config plugin's config() hook sets nextConfig (line 1849), and this plugin is registered after that one in the array, Vite processes config hooks in order — so nextConfig should be set by the time Nitro calls this hook. The guard is fine as a defensive check, but it silently no-ops if the assumption about hook ordering ever breaks. A console.warn might be worth adding so a misconfiguration doesn't silently disable route rules generation:

if (!nextConfig) {
  console.warn('[vinext] nextConfig not resolved — skipping Nitro routeRules generation');
  return;
}

Not blocking — just a suggestion for debuggability.

if (!hasAppDir && !hasPagesDir) return;

const { collectNitroRouteRules, mergeNitroRouteRules } =
await import("./build/nitro-route-rules.js");
const generatedRouteRules = await collectNitroRouteRules({
appDir: hasAppDir ? appDir : null,
pagesDir: hasPagesDir ? pagesDir : null,
pageExtensions: nextConfig.pageExtensions,
});

if (Object.keys(generatedRouteRules).length === 0) return;

const { routeRules, skippedRoutes } = mergeNitroRouteRules(
nitro.options.routeRules,
generatedRouteRules,
);

nitro.options.routeRules = routeRules;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: nitro.options.routeRules could theoretically be undefined (the NitroSetupContext type you defined on line 1175 declares it as routeRules?: Record<...>, i.e. optional). mergeNitroRouteRules handles this correctly via { ...existingRouteRules } (spreading undefined is a no-op), and the reassignment here is fine. Just noting that the types are consistent and this works.


if (skippedRoutes.length > 0) {
const warn = nitro.logger?.warn ?? console.warn;
warn(
`[vinext] Skipping generated Nitro routeRules for routes with existing exact cache config: ${skippedRoutes.join(", ")}`,
);
}
},
},
} as Plugin & { nitro: { setup: (nitro: NitroSetupContext) => Promise<void> } }, // Nitro plugin extension convention: https://nitro.build/guide/plugins
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: The trailing comment is a good call, but it would be clearer as a standalone comment above the object. Long inline comments after as casts are easy to miss.

Also consider whether nitro.options.routeRules could be undefined when the hook is called (Nitro initializes it, but the type allows it). The mergeNitroRouteRules function handles undefined correctly via { ...existingRouteRules }, so this is safe — just worth a note.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: this long inline comment after the as cast is easy to miss. Consider moving it to a standalone comment above the object literal.

Not blocking — just a readability suggestion.

// Vite can emit empty SSR manifest entries for modules that Rollup inlines
// into another chunk. Pages Router looks up assets by page module path at
// runtime, so rebuild those mappings from the emitted client bundle.
Expand Down
Loading
Loading