fix: map route segment revalidate to Nitro routeRules SWR

[Divkix]

Summary

• Add generateNitroRouteRules() to convert ISR routes (App Router export const revalidate = N and Pages Router getStaticProps with revalidate) to Nitro's routeRules format with swr values
• Add vinext:nitro-route-rules writeBundle hook that emits routeRules to .output/nitro.json when Nitro plugin is detected

Fixes

Closes #648

Testing

• All 9 new generateNitroRouteRules unit tests pass
• All 85 build-report tests pass
• All 176 routing + build-report tests pass

[pkg-pr-new]

Open in StackBlitz

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

commit: bb4e998

[Copilot]

Pull request overview

Adds Nitro integration to propagate Next.js ISR revalidate values into Nitro’s routeRules (as swr) so caching behavior is preserved in Nitro deployments.

Changes:

• Introduces generateNitroRouteRules() to convert build-report ISR rows into Nitro routeRules with swr.
• Adds a vinext:nitro-route-rules build hook that merges generated rules into .output/nitro.json when the Nitro plugin is detected.
• Adds unit tests for generateNitroRouteRules().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
tests/build-report.test.ts	Adds unit tests covering generateNitroRouteRules() mapping behavior.
packages/vinext/src/build/report.ts	Adds NitroRouteRules type and generateNitroRouteRules() implementation.
packages/vinext/src/index.ts	Adds a post-build hook to emit/merge ISR-derived routeRules into Nitro output.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

generateNitroRouteRules currently allows any number > 0, which would include Infinity. Infinity cannot be represented in JSON (JSON.stringify turns it into null), so writing these rules into .output/nitro.json would silently generate an invalid/incorrect config. Consider explicitly requiring Number.isFinite(row.revalidate) (and > 0) when generating SWR rules.

Suggested change

	if (row.type === "isr" && row.revalidate !== undefined && row.revalidate > 0) {
	if (
	row.type === "isr" &&
	typeof row.revalidate === "number" &&
	Number.isFinite(row.revalidate) &&
	row.revalidate > 0
	) {

[Copilot]

This test case constructs an ISR row with revalidate: Infinity, but classifyPagesRoute/classifyAppRoute classify revalidate === Infinity as static, so buildReportRows should never emit an isr row with Infinity. Keeping this test encourages behavior that will also serialize incorrectly to nitro.json (Infinity becomes null). Suggest removing this test or updating it to assert no rule is produced for Infinity.

Suggested change

	it("handles ISR routes with Infinity revalidate (treated as static)", () => {
	const rows = [
	{ pattern: "/static", type: "static" as const },
	{ pattern: "/isr", type: "isr" as const, revalidate: Infinity },
	];
	const result = generateNitroRouteRules(rows);
	expect(result).toEqual({
	"/isr": { swr: Infinity },
	});
	it("does not generate swr rules for ISR routes with Infinity revalidate (treated as static)", () => {
	const rows = [
	{ pattern: "/static", type: "static" as const },
	{ pattern: "/isr", type: "isr" as const, revalidate: Infinity },
	];
	const result = generateNitroRouteRules(rows);
	expect(result).toEqual({});

[Copilot]

RouteRow.pattern in vinext uses :param syntax (e.g. "/blog/:slug") rather than Next's bracket syntax ("/blog/[slug]"). Using bracket patterns in this test doesn't match real buildReportRows() output and could mask issues with routeRules generation for dynamic routes. Suggest updating the expected patterns to :slug (and :slug+ / :slug* for catch-alls) to reflect actual route patterns.

Suggested change

	{ pattern: "/blog/posts/[slug]", type: "isr" as const, revalidate: 20 },
	];
	expect(generateNitroRouteRules(rows)).toEqual({
	"/blog/posts": { swr: 10 },
	"/blog/posts/[slug]": { swr: 20 },
	{ pattern: "/blog/posts/:slug", type: "isr" as const, revalidate: 20 },
	];
	expect(generateNitroRouteRules(rows)).toEqual({
	"/blog/posts": { swr: 10 },
	"/blog/posts/:slug": { swr: 20 },

[Copilot]

Object.assign(nitroJson.routeRules, routeRules) replaces any existing per-route rule object wholesale. If Nitro (or user config) already set other fields for a route (e.g. headers/redirects/cors), they will be lost when an ISR rule is added. Consider merging per-route keys instead (e.g. keep existing rule object and only set/override the swr field).

Suggested change

	Object.assign(nitroJson.routeRules, routeRules);
	for (const [route, rule] of Object.entries(routeRules)) {
	nitroJson.routeRules[route] = {
	...(nitroJson.routeRules[route] ?? {}),
	...rule,
	};
	}

[Copilot]

The nitro.json parse error is silently ignored and then the file is rewritten, which can clobber an existing Nitro config (dropping fields other than routeRules) if the JSON is temporarily invalid/partially written. Consider failing the build or at least logging a warning and skipping the write when parsing fails, to avoid producing a broken .output/nitro.json.

Suggested change

	if (fs.existsSync(nitroOutputPath)) {
	try {
	nitroJson = JSON.parse(fs.readFileSync(nitroOutputPath, "utf-8"));
	} catch {
	// ignore parse errors
	}
	}
	let nitroJsonParseError = false;
	if (fs.existsSync(nitroOutputPath)) {
	try {
	nitroJson = JSON.parse(fs.readFileSync(nitroOutputPath, "utf-8"));
	} catch (error) {
	nitroJsonParseError = true;
	console.warn(
	`[vinext] Failed to parse existing nitro.json at ${nitroOutputPath}:`,
	error instanceof Error ? error.message : error,
	);
	}
	}
	if (nitroJsonParseError) {
	// Skip writing to avoid clobbering an existing but temporarily invalid nitro.json
	return;
	}

[james-elicx]

/bigbonk review

[ask-bonk]

Review

Good feature — mapping ISR revalidate values to Nitro's swr is the right approach for Nitro deployment parity. The generateNitroRouteRules function is clean and well-guarded (with Number.isFinite and > 0 checks). I see the Copilot feedback on merge behavior and parse-error handling has already been addressed. A few issues remain.

Summary of issues

1. Duplicate execution (medium): The writeBundle hook fires for both rsc and ssr environments. In App Router builds (where hasNitroPlugin implies isMultiEnv), this re-scans all routes and re-writes nitro.json twice — wasted work and a potential ordering issue if Nitro hasn't finished its own writeBundle between the two calls.
2. Missing mkdirSync for .output/ (medium): If Nitro hasn't created .output/ before this hook runs, fs.writeFileSync throws ENOENT. Need fs.mkdirSync(path.dirname(nitroOutputPath), { recursive: true }) before the write.
3. Route pattern format mismatch (needs investigation): vinext patterns use :param syntax (/blog/:slug), but Nitro routeRules use glob-style patterns (/blog/**). Passing vinext-internal patterns directly may not work as expected in Nitro's route matching. This should be validated against Nitro's route matching behavior.
4. No integration test for the writeBundle hook: The unit tests for generateNitroRouteRules are solid, but there's no test verifying the writeBundle hook actually writes correct nitro.json output (file I/O, merge logic, directory creation). Consider at minimum a test that mocks fs or uses a temp directory.

[ask-bonk]

This hook runs for both rsc and ssr environments. In an App Router + Nitro build, both environments exist (since hasNitroPlugin sets isMultiEnv = true), so this hook will fire twice — re-scanning all routes and re-writing nitro.json redundantly.

The other writeBundle hooks that accept both environments (like vinext:server-manifest) do so intentionally because they need the options.dir which differs per environment. This hook doesn't use options.dir at all — it writes to a fixed .output/nitro.json path.

Pick one environment to gate on (probably rsc since App Router is the primary target, and for Pages Router only ssr exists):

Suggested change

	if (envName !== "rsc" && envName !== "ssr") return;
	if (envName !== "rsc" && envName !== "ssr") return;
	// Avoid running twice in multi-environment builds (App Router has both
	// rsc and ssr). Prefer rsc; fall back to ssr for Pages Router builds.
	if (envName === "ssr" && this.environment?.config?.environments?.rsc) return;

Alternatively, a simpler approach: just gate on rsc || (ssr && !hasAppDir) using the existing hasAppDir closure variable.

[ask-bonk]

If .output/ doesn't exist yet when this hook runs (possible if Nitro's own writeBundle hasn't executed first), this will throw ENOENT. Add a mkdirSync call before writing:

Suggested change

	fs.writeFileSync(nitroOutputPath, JSON.stringify(nitroJson, null, 2));
	fs.mkdirSync(path.dirname(nitroOutputPath), { recursive: true });
	fs.writeFileSync(nitroOutputPath, JSON.stringify(nitroJson, null, 2));

[ask-bonk]

Nit: any[] loses type safety. The router functions return typed arrays (AppRoute[] and Route[]). Consider:

Suggested change

	let appRoutes: any[] = [];
	let appRoutes: Awaited<ReturnType<typeof appRouter>> = [];
	let pageRoutes: Awaited<ReturnType<typeof pagesRouter>> = [];
	let apiRoutes: Awaited<ReturnType<typeof apiRouter>> = [];

Or import the AppRoute and Route types from the routing modules.

[ask-bonk]

This passes vinext's internal route pattern (:param syntax like /blog/:slug) directly as the Nitro routeRules key. Nitro uses its own route matching which expects glob-style patterns (/blog/**) or exact paths.

For static ISR routes (no dynamic segments) this works fine. But for dynamic ISR routes like /blog/:slug, Nitro won't recognize the :slug parameter syntax. You'll likely need a conversion step here, e.g.:

• /blog/:slug → /blog/**
• /docs/:slug+ → /docs/**
• /docs/:slug* → /docs/**

Please verify against Nitro's routeRules docs to confirm the expected format. If this is intentionally deferred, add a TODO comment.

[ask-bonk]

Good that you test the Infinity edge case after incorporating the Number.isFinite guard. Worth noting in a comment that this case should never arise from buildReportRows in practice (since classifyAppRoute maps revalidate === Infinity to type: "static"), but it's still valid as a defensive test for generateNitroRouteRules in isolation.

Suggested change

	{ pattern: "/isr", type: "isr" as const, revalidate: Infinity },
	// In practice, buildReportRows never produces an ISR row with Infinity
	// (classifyAppRoute maps Infinity to "static"), but generateNitroRouteRules
	// should handle it defensively since Infinity serializes to null in JSON.
	{ pattern: "/isr", type: "isr" as const, revalidate: Infinity },

[ask-bonk]

Review posted successfully. Here's a summary of the feedback I left on PR #669:

Key issues raised

1. Duplicate execution — The writeBundle hook fires for both rsc and ssr environments, causing redundant route scanning and double-writing of nitro.json in App Router builds. Suggested gating to run only once per build.

2. Missing .output/ directory creation — fs.writeFileSync will throw ENOENT if .output/ doesn't exist yet when the hook runs. Needs mkdirSync with { recursive: true }.

3. Route pattern format mismatch — vinext uses :param syntax (/blog/:slug) but Nitro's routeRules expect glob-style patterns (/blog/**). Dynamic ISR routes will likely not match correctly in Nitro without a conversion step. Flagged for investigation/validation.

4. Type safety — The any[] variable declarations for routes lose type information unnecessarily.

5. Test comment clarification — Noted that the Infinity test case is a valid defensive test but should document that buildReportRows never produces this combination in practice.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Review

Good idea — mapping ISR revalidate to Nitro swr routeRules is the right approach. The generateNitroRouteRules function itself is clean and well-guarded. The previous review feedback (duplicate execution, mkdirSync, merge semantics, parse error handling) has been incorporated. However, there are two issues that need to be addressed before this is mergeable.

Issues

1. Route pattern format mismatch (blocking) — The comment on line 846 claims "Nitro uses rou3" but Nitro actually uses radix3 (the older library) with toRouteMatcher. While :param syntax technically works in radix3's route matcher, it's undocumented for routeRules. The documented and conventional format is /** glob patterns for dynamic segments. This needs investigation and either a conversion step or explicit validation that :param syntax works correctly with the specific Nitro version being targeted.

2. Redundant route scanning at build time (design concern) — The writeBundle hook re-scans app/ and pages/ directories from scratch, even though the exact same scanning already happened during the build (for the build report in cli.ts). This is unnecessary I/O and file-system analysis. Consider either (a) caching the route rows from the build report step and passing them through, or (b) at minimum documenting why the re-scan is intentional.

3. Top-level import of build/report.js in index.ts — The main plugin file now eagerly imports buildReportRows, generateNitroRouteRules, and findDir from build/report.js. Note that cli.ts deliberately uses dynamic import() for printBuildReport to avoid loading build-report code during dev. This new static import means the entire report module (including classifyAppRoute, classifyPagesRoute, regex patterns, etc.) is loaded at plugin initialization time, including in dev mode where it's never used. Consider using a dynamic import inside the writeBundle handler instead.

[ask-bonk]

This comment is inaccurate. Nitro uses radix3 (specifically toRouteMatcher from radix3), not rou3. While :param syntax technically works in radix3's internal parser (it creates PLACEHOLDER nodes), it is undocumented for Nitro's routeRules. All Nitro documentation and examples use either exact paths (/blog) or glob patterns (/blog/**).

The :param syntax also has a subtle semantic difference: /blog/:slug matches exactly one segment (/blog/foo but not /blog/foo/bar), while /blog/** matches any depth. For ISR routes with dynamic segments, the glob form is likely what users expect.

Please either:

1. Add a conversion step (:slug → /**, :slug+ → /**, etc.) and update the comment, or
2. Validate that :param syntax works correctly with the specific Nitro/radix3 version and document the intentional choice with a link to supporting evidence

Suggested change

	// Patterns like /blog/:slug work directly in Nitro routeRules without conversion.
	// Note: Nitro uses radix3's toRouteMatcher for routeRules matching. While radix3
	// internally parses :param syntax, Nitro's documented format uses exact paths ("/blog")
	// and glob patterns ("/blog/**"). Static ISR routes (no dynamic segments) work as-is.
	// TODO: Dynamic ISR routes need pattern conversion (:slug → /**) for Nitro compatibility.

[ask-bonk]

This is a new static import that pulls in the entire build/report.js module at plugin initialization time. In cli.ts, printBuildReport is deliberately loaded via dynamic import() (line 900 of report.ts) to avoid loading build/report code during dev.

Since this code is only needed in the writeBundle hook (build-only), consider using a dynamic import inside the handler instead:

const { buildReportRows, generateNitroRouteRules, findDir } = await import("./build/report.js");

This keeps the dev server lean.

[ask-bonk]

This re-scans app/ and pages/ directories and re-reads every route file's source code to extract revalidate values — the same work that printBuildReport already does in cli.ts after the build completes. In a large project this could add noticeable time to the build.

Consider storing the buildReportRows result from the build report step (or from an earlier build phase) and making it available to this hook, rather than re-scanning from scratch. Even a simple module-level cache (set by the build report, read by this hook) would eliminate the duplicate work.

If re-scanning is intentional (e.g., because the hook runs in a different process or the ordering isn't guaranteed), a comment explaining why would be helpful.

[ask-bonk]

This mergeNitroRouteRules helper reimplements the merge logic from index.ts but with different error handling behavior — it silently ignores parse errors (line 873-874), while the actual hook warns and returns early. The tests exercise this local copy, not the real code path.

This means:

• The mkdirSync fix in the actual hook isn't tested (this helper has its own mkdirSync)
• The parse-error-and-bail behavior in the actual hook isn't tested
• If someone changes the merge logic in index.ts, these tests still pass

Consider extracting the merge logic into a shared, testable function (e.g., mergeNitroRouteRules in build/report.ts) that both the writeBundle hook and these tests use. That way the tests exercise the real code.

[ask-bonk]

Minor: this.environment?.config?.root returns the environment-specific root, which should be the same as the project root in normal builds. But for clarity and consistency with how printBuildReport works (it receives root explicitly in cli.ts), consider using the resolved config root that's already available in the plugin closure. Is there a root variable available from the configResolved hook?

[ask-bonk]

Review posted successfully. Here's a summary of the feedback I left on PR #669:

Key issues raised

1. Route pattern format mismatch (blocking) — The comment claims "Nitro uses rou3" but Nitro actually uses radix3 with toRouteMatcher. While :param syntax technically works in radix3's internal parser, it's undocumented for Nitro's routeRules. All Nitro docs/examples use exact paths or /** glob patterns. Dynamic ISR routes (e.g., /blog/:slug) need a conversion step to /blog/** or explicit validation.

2. Redundant route scanning — The writeBundle hook re-scans app/ and pages/ directories from scratch, duplicating the same work that printBuildReport already does in cli.ts. Suggested caching or sharing the results.

3. Top-level import bloat — build/report.js is now statically imported in index.ts, pulling the entire report module (classification regexes, etc.) into the dev server where it's never used. cli.ts deliberately uses dynamic import() for this. Suggested using dynamic import inside the writeBundle handler.

4. Test isolation concern — The file I/O tests reimplement the merge logic locally (mergeNitroRouteRules helper) with different error-handling behavior than the actual hook. Tests don't exercise the real code path. Suggested extracting the merge logic into a shared testable function.

5. Minor: config root — Suggested using the plugin closure's resolved root instead of this.environment?.config?.root.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Review

Good improvement over the earlier writeBundle approach. Using Nitro's nitro.setup hook to mutate nitro.options.routeRules directly is the correct integration pattern — it avoids all the file I/O issues (missing .output/, duplicate execution across environments, nitro.json merge races) that prior reviews flagged. The generateNitroRouteRules and mergeNitroRouteRules functions are clean, well-guarded, and properly tested.

Two issues remain — one blocking, one design-level.

Blocking: Route pattern format mismatch

vinext uses :param syntax internally (/blog/:slug, /docs/:slug+), but Nitro's routeRules use radix3's toRouteMatcher for matching. All Nitro documentation and examples use exact paths (/blog) or glob patterns (/blog/**). The :param syntax is an internal detail of radix3's parser that is not documented for routeRules use.

This means:

• Static ISR routes (/, /about) work fine — they're exact matches.
• Dynamic ISR routes (/blog/:slug) may not match correctly at runtime.

This needs either a conversion step (:slug → /**, :slug+ → /**, :slug* → /**) or explicit validation that :param syntax works end-to-end with the Nitro version being targeted.

Design: Redundant route scanning

collectNitroRouteRules re-scans app/ and pages/ directories and re-reads every route file's source code to extract revalidate — the exact same work that printBuildReport does in cli.ts after the build. For large projects this doubles the filesystem analysis.

The nitro.setup hook runs during Nitro initialization (before the build), while printBuildReport runs after the build completes, so the timing makes sharing tricky. But it's worth noting as a future optimization — a build-phase cache that stores route classification results would eliminate the duplication.

Nits

See inline comments.

[ask-bonk]

This passes vinext's internal route pattern (:param syntax) directly as the Nitro routeRules key. For static routes like / this works fine, but for dynamic routes like /blog/:slug, Nitro's radix3-based toRouteMatcher may not recognize this syntax.

Nitro's documented routeRules format uses exact paths or /** glob patterns. A conversion step is needed here:

• /blog/:slug → /blog/**
• /docs/:slug+ → /docs/**
• /docs/:slug* → /docs/**

Alternatively, validate that :param syntax works correctly end-to-end with Nitro's route matcher and document the finding.

Suggested change

	rules[row.pattern] = { swr: row.revalidate };
	rules[convertToNitroPattern(row.pattern)] = { swr: row.revalidate };

Where convertToNitroPattern replaces dynamic segments:

function convertToNitroPattern(pattern: string): string {
  // Replace :param, :param+, :param* segments with ** glob
  return pattern.replace(/\/:([a-zA-Z_][a-zA-Z0-9_-]*)([+*]?)(\/|$)/g, '/**$3');
}

[ask-bonk]

This test name says "preserves exact vinext route patterns for Nitro rules" which reads as an intentional design choice. But the question of whether Nitro actually understands :param syntax in routeRules hasn't been validated (see comment on nitro-route-rules.ts:52). If a conversion step is added, this test should verify the conversion output instead.

[ask-bonk]

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.

[ask-bonk]

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.

[ask-bonk]

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.

[ask-bonk]

The as Plugin & { nitro: ... } cast is necessary since Vite's Plugin type doesn't know about Nitro's extension. This is fine, but worth adding a brief comment explaining why the cast exists — future readers may not know about Nitro's plugin extension convention.

Suggested change

	} as Plugin & { nitro: { setup: (nitro: NitroSetupContext) => Promise<void> } },
	} as Plugin & { nitro: { setup: (nitro: NitroSetupContext) => Promise<void> } }, // Nitro plugin extension convention: https://nitro.build/guide/plugins

[ask-bonk]

This initializeNitroSetupPlugin helper calls configPlugin.config() which sets up hasAppDir, pagesDir, nextConfig, etc. in the plugin closure. Nice approach for integration testing.

One concern: this passes { root, plugins: [] } with an empty plugins array, which means hasNitroPlugin will be false in the closure. The nitro.setup hook doesn't check hasNitroPlugin (it doesn't need to — if Nitro calls the hook, Nitro is present), so this works. But if someone adds a hasNitroPlugin guard to the hook later, these tests would silently stop exercising the code path. Consider adding a comment noting this.

Suggested change

	const plugins = vinext({ appDir: root, rsc: false }) as ReturnType<typeof vinext>;
	// Note: passing empty plugins array means hasNitroPlugin=false in the closure,
	// but the nitro.setup hook doesn't gate on hasNitroPlugin (Nitro calls it directly).
	await configPlugin.config({ root, plugins: [] }, { command: "build", mode: "production" });

[ask-bonk]

Good defensive test. Worth adding a comment that buildReportRows never produces this combination in practice (since classifyAppRoute maps revalidate === Infinity to type: "static"), so this is purely testing generateNitroRouteRules's own guard.

Suggested change

	it("ignores Infinity revalidate defensively", () => {
	// In practice, buildReportRows never produces an ISR row with Infinity
	// (classifyAppRoute maps Infinity to "static"), but generateNitroRouteRules
	// should handle it defensively since Infinity serializes to null in JSON.
	it("ignores Infinity revalidate defensively", () => {

[ask-bonk]

Review posted successfully. Here's a summary of the feedback:

Key issues raised

1. Route pattern format mismatch (blocking) — vinext uses :param syntax (/blog/:slug) internally, but Nitro's routeRules use radix3's toRouteMatcher which documents exact paths and /** glob patterns. Dynamic ISR routes need a conversion step (:slug → /**) or explicit validation that :param syntax works end-to-end with Nitro.

2. Redundant route scanning (design) — collectNitroRouteRules re-scans the filesystem, duplicating work printBuildReport already does. Since nitro.setup runs before the build while printBuildReport runs after, sharing is non-trivial, but noted as a future optimization.

3. Positive notes — The switch from writeBundle to nitro.setup is the right approach. The import type on line 12 is correctly type-only (prior review concern about import bloat was wrong). The generateNitroRouteRules and mergeNitroRouteRules functions are well-structured.

4. Minor items — Suggested documenting the Nitro plugin cast, adding defensive comments on the Infinity test case, noting the missing prerenderResult pass-through, and documenting the hasNitroPlugin=false state in test setup.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Review

Good feature — mapping ISR revalidate values to Nitro swr routeRules via the nitro.setup hook is the right integration pattern. The code is cleanly structured: collectNitroRouteRules, generateNitroRouteRules, mergeNitroRouteRules, and convertToNitroPattern are well-separated concerns. The merge logic correctly preserves user-defined cache rules and warns on skips. The dynamic import in the hook avoids loading build-report code during dev. Tests are thorough.

One blocking bug and a couple of smaller issues.

Blocking: convertToNitroPattern regex fails on consecutive dynamic segments

The regex consumes the trailing / delimiter, which prevents matching the next dynamic segment. This means routes like /:category/:id or /blog/:year/:month/:slug are only partially converted:

/:a/:b                      -> "/**/:b"         (expected: "/**/**")
/blog/:year/:month/:slug    -> "/blog/**/:month/**"  (expected: "/blog/**/**/**")
/api/:resource/:id          -> "/api/**/:id"    (expected: "/api/**/**")

These are real patterns produced by both the App Router and Pages Router scanners. The fix is to use a lookahead instead of a capturing group for the trailing delimiter — see inline suggestion.

Minor issues

1. mergeNitroRouteRules tests use pre-conversion :slug patterns — The tests for mergeNitroRouteRules pass :slug patterns as keys, but in the real code path, generateNitroRouteRules calls convertToNitroPattern first, so mergeNitroRouteRules would receive /** glob keys. The tests work because mergeNitroRouteRules is key-agnostic, but they could be misleading about the actual runtime key format. Consider using /** patterns in these tests for consistency.

2. Missing test for consecutive dynamic segments — The convertToNitroPattern tests only cover single-segment conversions. After fixing the regex, add a test case for consecutive segments (e.g., /:a/:b → /**/**).

[ask-bonk]

Bug: The regex captures the trailing / in group $3, consuming it from the input. When two dynamic segments are adjacent (/:a/:b), the / between them is consumed by the first match, so the second segment's leading / is gone and the global regex doesn't see /:b — it sees just b.

Verified:

/:a/:b                   -> "/**/:b"           (should be "/**/**")
/blog/:year/:month/:slug -> "/blog/**/:month/**" (should be "/blog/**/**/**")
/api/:resource/:id       -> "/api/**/:id"      (should be "/api/**/**")

Fix: use a lookahead (?=\/|$) instead of a capturing group so the delimiter is asserted but not consumed:

Suggested change

	return pattern.replace(/\/:([a-zA-Z_][a-zA-Z0-9_-]*)([+*]?)(\/|$)/g, "/**$3");
	return pattern.replace(/\/:([a-zA-Z_][a-zA-Z0-9_-]*)([+*]?)(?=\/|$)/g, "/**");

[ask-bonk]

This test case (/users/:id/posts) has a static segment between the dynamics, so it doesn't exercise the consecutive-segment bug. After fixing the regex, add a test for consecutive dynamic segments to prevent regressions:

Suggested change

	});
	expect(convertToNitroPattern("/users/:id/posts")).toBe("/users/**/posts");
	expect(convertToNitroPattern("/:category/:id")).toBe("/**/**");
	expect(convertToNitroPattern("/blog/:year/:month/:slug")).toBe("/blog/**/**/**");

[ask-bonk]

Nit: In the real code path, generateNitroRouteRules converts patterns with convertToNitroPattern before they reach mergeNitroRouteRules. So at merge time, keys would be "/blog/**" not "/blog/:slug". These tests are technically correct (merge is key-agnostic), but using post-conversion patterns would better document the actual runtime behavior and catch key-format mismatches.

[ask-bonk]

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.

[ask-bonk]

Review posted successfully. Here's a summary of the feedback:

Key issue (blocking)

convertToNitroPattern regex bug with consecutive dynamic segments — The regex at nitro-route-rules.ts:83 captures the trailing / delimiter, consuming it from the input string. When two dynamic segments are adjacent (e.g., /:a/:b), the first match eats the / that the second segment needs as its prefix, so every other dynamic segment is silently left unconverted. This is verified with real patterns that both routers produce:

• /:a/:b → "/**/:b" (should be "/**/**")
• /blog/:year/:month/:slug → "/blog/**/:month/**" (should be "/blog/**/**/**")

Fix: use a lookahead (?=\/|$) instead of a capturing group (\/|$).

Minor items

1. mergeNitroRouteRules tests use pre-conversion :slug patterns as keys, but in the real code path those keys would already be /** globs. Suggested using post-conversion patterns for clarity.
2. Missing test coverage for consecutive dynamic segments in the convertToNitroPattern suite.
3. Minor formatting note on the inline type cast comment in index.ts:4416.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Review

The PR is in much better shape after 4 rounds of review feedback. The major issues from prior reviews (duplicate execution via writeBundle, mkdirSync, regex bug on consecutive segments, import bloat) are all resolved. The nitro.setup hook is the correct integration point, convertToNitroPattern uses a clean split/map/join approach, the dynamic import avoids dev-time overhead, and mergeNitroRouteRules correctly preserves user-defined cache rules.

Two items to address — one correctness issue and one doc fix.

1. Pattern conversion over-matches: :param maps to ** but should map to *

Nitro uses rou3 which distinguishes between:

• * — single-segment wildcard (matches /blog/foo but NOT /blog/foo/bar)
• ** — multi-segment wildcard (matches /blog/foo, /blog/foo/bar, etc.)

The current convertToNitroPattern maps all dynamic segments to **, but vinext's :param (without + or * suffix) only matches a single segment. The correct mapping is:

• :param → * (single segment)
• :param+ → ** (one or more segments — catch-all)
• :param* → ** (zero or more segments — optional catch-all)

In practice this is low-severity since over-matching an SWR cache rule only applies caching to paths that would 404 anyway. But it's semantically wrong and could cause confusion when debugging cache behavior. See inline suggestion.

2. Doc comment references radix3 but Nitro uses rou3

Minor but the comment on line 74 says "radix3's toRouteMatcher". Nitro migrated to rou3 (the rename of radix3, now under the h3js org). The function is now just createRouter + findAllRoutes from rou3. See inline suggestion.

Everything else looks good

• generateNitroRouteRules guards correctly (Number.isFinite, > 0)
• mergeNitroRouteRules merge semantics are correct — spread preserves non-cache fields, hasUserDefinedCacheRule skips routes with explicit cache config
• collectNitroRouteRules doc comment correctly explains the prerenderResult gap
• Integration tests exercise the real plugin closure via initializeNitroSetupPlugin
• The Infinity test has a clear explanatory comment
• import type on line 12 of index.ts is correctly type-only (zero runtime cost)

[ask-bonk]

: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

[ask-bonk]

The comment references radix3 but Nitro now uses rou3 (the successor, under the h3js org). toRouteMatcher is also no longer the relevant API — Nitro uses createRouter/findAllRoutes from rou3, compiled via rou3/compiler.

Suggested change

	/**
	* Converts vinext's internal `:param` route syntax to Nitro's `/**` glob
	* pattern format. Nitro's `routeRules` use radix3's `toRouteMatcher` which
	* documents exact paths and `/**` globs, not `:param` segments.
	/**
	* Converts vinext's internal `:param` route syntax to Nitro's glob
	* pattern format. Nitro uses rou3 for routeRules matching, which
	* supports `*` (single-segment) and `**` (multi-segment) wildcards.
	*

[ask-bonk]

These test cases would need updating after fixing convertToNitroPattern to use * for single-segment params. With the fix, bare :param segments should produce * not **:

Suggested change

	it("handles consecutive dynamic segments correctly", () => {
	expect(convertToNitroPattern("/:a/:b")).toBe("/**/**");
	expect(convertToNitroPattern("/blog/:year/:month/:slug")).toBe("/blog/**/**/**");
	expect(convertToNitroPattern("/api/:version/:resource/:id")).toBe("/api/**/**/**");
	});
	it("handles consecutive dynamic segments correctly", () => {
	expect(convertToNitroPattern("/:a/:b")).toBe("/*/*");
	expect(convertToNitroPattern("/blog/:year/:month/:slug")).toBe("/blog/*/*/*");
	expect(convertToNitroPattern("/api/:version/:resource/:id")).toBe("/api/*/*/*");
	});

Also consider adding a mixed test case to verify the distinction:

expect(convertToNitroPattern("/blog/:year/:slug+")).toBe("/blog/*/**");

[ask-bonk]

After the * vs ** fix, these should become:

Suggested change

	it("converts :param segments to /** globs", () => {
	expect(convertToNitroPattern("/blog/:slug")).toBe("/blog/**");
	expect(convertToNitroPattern("/users/:id/posts")).toBe("/users/**/posts");
	it("converts :param segments to /* globs", () => {
	expect(convertToNitroPattern("/blog/:slug")).toBe("/blog/*");
	expect(convertToNitroPattern("/users/:id/posts")).toBe("/users/*/posts");
	});

[ask-bonk]

Nit: the backtick escaping in the JSDoc makes this hard to read. Since this is a code comment (not rendered markdown), the raw text is fine:

Suggested change

	* /:a/:b -> /`**`/`**` (consecutive segments)
	* /:a/:b -> /*/* (consecutive segments)

[ask-bonk]

Review posted successfully. Here's a summary of the feedback on PR #669:

Key issues raised

1. Pattern conversion semantic mismatch (correctness) — convertToNitroPattern maps all dynamic segments (:param, :param+, :param*) uniformly to **, but rou3 distinguishes between * (single-segment) and ** (multi-segment). The correct mapping is :param → *, :param+ → **, :param* → **. Low severity in practice (over-matching SWR on paths that 404 is harmless) but semantically wrong and could confuse debugging.

2. Doc comment references wrong library — Comment says "radix3's toRouteMatcher" but Nitro now uses rou3 (the successor under the h3js org) with createRouter/findAllRoutes.

Positive notes

• The nitro.setup hook is the correct integration pattern (prior writeBundle issues are resolved)
• convertToNitroPattern split/map/join approach correctly handles consecutive dynamic segments (the blocking regex bug from the prior review is fixed)
• import type is correctly type-only (no runtime overhead)
• Dynamic import in the hook keeps dev server lean
• mergeNitroRouteRules correctly preserves user-defined cache rules
• Test coverage is thorough, including integration tests via initializeNitroSetupPlugin

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Now I have a thorough understanding. Let me write the review.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Review

This PR has been through 6 rounds of automated review and iterated well. The architecture is sound: using nitro.setup to mutate nitro.options.routeRules directly is the correct integration pattern. The code is cleanly factored into collectNitroRouteRules, generateNitroRouteRules, mergeNitroRouteRules, and convertToNitroPattern. The import type avoids runtime overhead, the dynamic import in the hook keeps the dev server lean, Number.isFinite guards against Infinity serialization, and the merge logic correctly preserves user-defined cache rules. CI is fully green.

Two small items remain — one is a documentation bug (garbled JSDoc), the other is a nit about the NitroSetupContext type not accounting for routeRules being potentially undefined. Neither is blocking.

Overall this looks good to merge.

[ask-bonk]

The backtick-escaped example is garbled. It renders as `/*`/`/*` instead of the intended /*/*. Since this is a JSDoc code comment (not rendered markdown), the backticks aren't needed and actively make it harder to read.

Suggested change

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

[ask-bonk]

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.

[ask-bonk]

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.

[ask-bonk]

Good test architecture — calling configPlugin.config() to set up the closure state before exercising nitro.setup is a clean way to integration-test the plugin without a full Vite build. The comment about hasNitroPlugin=false is helpful.

[ask-bonk]

Review submitted. Here's a summary of the feedback:

Overall assessment: This PR is in good shape and ready to merge. The architecture is sound — using nitro.setup to mutate nitro.options.routeRules directly is the correct integration pattern, avoiding all the file I/O issues (missing .output/, duplicate execution, merge races) that plagued earlier iterations.

Inline comments posted:

1. Garbled JSDoc on line 81 — The backtick-escaped /:a/:b example renders as broken markdown. Suggested removing the unnecessary backticks since it's a code comment, not rendered markdown.

2. Silent no-op on !nextConfig guard — The nextConfig check at line 4388 silently returns if the hook ordering assumption ever breaks. Suggested adding a console.warn for debuggability (not blocking).

3. Noted that nitro.options.routeRules being optional in the type is handled correctly by mergeNitroRouteRules.

4. Positive note on the integration test approach — calling configPlugin.config() to set up closure state before exercising nitro.setup is a clean pattern.

github run

[james-elicx]

/bigbonk review

[ask-bonk]

Review

This PR is in good shape after 6+ rounds of iteration. The architecture is correct, the code is clean, and CI is fully green.

What works well:

• Using nitro.setup to mutate nitro.options.routeRules directly is the right integration pattern — avoids all the file I/O, duplicate execution, and race condition issues from earlier iterations
• Clean separation of concerns: collectNitroRouteRules, generateNitroRouteRules, mergeNitroRouteRules, convertToNitroPattern
• import type for NitroRouteRuleConfig has zero runtime cost
• Dynamic import() inside the hook keeps build-report code out of the dev server
• Number.isFinite + > 0 guards handle the Infinity/JSON serialization edge case
• mergeNitroRouteRules correctly preserves user-defined cache rules via hasUserDefinedCacheRule
• convertToNitroPattern split/map/join approach correctly handles consecutive dynamic segments and distinguishes * vs **
• Integration tests exercise the real plugin closure via initializeNitroSetupPlugin — good pattern
• Test coverage is thorough: unit tests for each exported function, filesystem-based collectNitroRouteRules tests for both routers, and integration tests for the plugin hook

Two minor items inline — a garbled JSDoc line and a nit on the trailing inline comment. Neither is blocking.

[ask-bonk]

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)

[ask-bonk]

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.

[ask-bonk]

Review submitted. Here's a summary of the feedback:

Approved — This PR is ready to merge. After 6+ rounds of automated review and iteration, all major issues have been resolved:

• Architecture is sound: nitro.setup hook is the correct integration point
• convertToNitroPattern correctly handles consecutive dynamic segments and distinguishes * (single-segment) vs ** (catch-all)
• import type has zero runtime cost, dynamic import() keeps dev server lean
• mergeNitroRouteRules correctly preserves user-defined cache rules
• Test coverage is thorough across all exported functions and the plugin integration

Two minor inline items (non-blocking):

1. Garbled JSDoc on line 81 of nitro-route-rules.ts — backtick-escaped /* fragments render as broken text. Suggested removing unnecessary backticks.
2. Long inline comment on line 4416 of index.ts after the as cast — suggested moving to a standalone comment for readability.

github run