-
Notifications
You must be signed in to change notification settings - Fork 309
fix: map route segment revalidate to Nitro routeRules SWR #669
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.
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
Changes from all commits
7e8701d
b5edd23
ef0597a
6edb174
247890f
5ff1d4e
ae50317
14c106e
54d4cea
ba0751b
bb4e998
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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> & { | ||||||||||||||||||||||||||||||||||||||||||||||||
| 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 })); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Also, |
||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is garbled — the backtick-escaped
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This means The fix is to distinguish between single-segment params and catch-all params:
Suggested change
And update the tests accordingly:
|
||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good — |
||
| import { createValidFileMatcher } from "./routing/file-matcher.js"; | ||
| import { createSSRHandler } from "./server/dev-server.js"; | ||
| import { handleApiRoute } from "./server/api-handler.js"; | ||
|
|
@@ -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; | ||
|
|
@@ -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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: 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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: |
||
|
|
||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Also consider whether
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this long inline comment after the 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. | ||
|
|
||
There was a problem hiding this comment.
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'sNitroRouteConfig.