-
Notifications
You must be signed in to change notification settings - Fork 327
Add native App Router route type generation #1144
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
ff0c7bf
b6e5246
9f9fc58
f4300b4
4b45538
1f3ea02
d53d7d8
98f515a
e323f6d
5a8b06c
eb1acff
09ccb8e
3b78188
331ad5d
30e81da
74f16cc
8edcb50
60117bb
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 |
|---|---|---|
|
|
@@ -165,21 +165,41 @@ jobs: | |
| working-directory: ${{ runner.temp }}/cna-test | ||
| shell: bash | ||
| run: | | ||
| vp exec vite dev --port 3099 & | ||
| vp exec vite dev --host 127.0.0.1 --port 3099 & | ||
|
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 improvement — explicit
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 reliability fix — explicit |
||
| SERVER_PID=$! | ||
| trap 'kill "$SERVER_PID" 2>/dev/null || true' EXIT | ||
|
|
||
| for i in $(seq 1 30); do | ||
| STATUS=$(curl -s -o /dev/null -w "%{http_code}" http://localhost:3099/ || true) | ||
| for i in $(seq 1 45); do | ||
| STATUS=$(node -e ' | ||
| const http = require("node:http"); | ||
| let done = false; | ||
| function finish(status) { | ||
| if (done) return; | ||
| done = true; | ||
| console.log(status); | ||
| process.exit(status === 200 ? 0 : 1); | ||
| } | ||
| const req = http.get("http://127.0.0.1:3099/", (res) => { | ||
| res.resume(); | ||
| finish(res.statusCode); | ||
| }); | ||
| req.setTimeout(10000, () => { | ||
| req.destroy(); | ||
| finish("timeout"); | ||
| }); | ||
| req.on("error", () => { | ||
| finish("000"); | ||
| }); | ||
| ' || true) | ||
| echo "HTTP status from dev server: ${STATUS:-request failed} (attempt $i)" | ||
| if [ "$STATUS" = "200" ]; then | ||
| echo "Server responded with HTTP 200 (attempt $i)" | ||
| kill "$SERVER_PID" 2>/dev/null || true | ||
| exit 0 | ||
| fi | ||
| sleep 1 | ||
| done | ||
|
|
||
| echo "Server did not respond with HTTP 200 within 30 seconds" | ||
| kill "$SERVER_PID" 2>/dev/null || true | ||
| echo "Server did not respond with HTTP 200 before the retry limit" | ||
| exit 1 | ||
|
|
||
| e2e: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ type ParsedArgs = { | |
| prerenderAll?: boolean; | ||
| prerenderConcurrency?: number; | ||
| precompress?: boolean; | ||
| positionals?: string[]; | ||
| }; | ||
|
|
||
| // Matches long flags (--foo) and single-letter short flags (-x). | ||
|
|
@@ -92,6 +93,11 @@ export function parsePositiveIntegerArg(raw: string, flag: string): number { | |
| */ | ||
| export function parseArgs(args: string[]): ParsedArgs { | ||
| const result: ParsedArgs = {}; | ||
| const addPositional = (arg: string): void => { | ||
| result.positionals ??= []; | ||
| result.positionals.push(arg); | ||
| }; | ||
|
|
||
| for (let i = 0; i < args.length; i++) { | ||
| const arg = args[i]; | ||
|
|
||
|
|
@@ -166,6 +172,9 @@ export function parseArgs(args: string[]): ParsedArgs { | |
| ); | ||
| break; | ||
| } | ||
| if (!FLAG_PATTERN.test(arg)) { | ||
|
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 guard correctly skips flag-looking strings (so Not blocking for this PR.
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: unknown flags like |
||
| addPositional(arg); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| * vinext build Build for production | ||
| * vinext start Start production server | ||
| * vinext deploy Deploy to Cloudflare Workers | ||
| * vinext typegen Generate App Router route helper types | ||
| * vinext lint Run linter (delegates to eslint/oxlint) | ||
| * | ||
| * Automatically configures Vite with the vinext plugin — no vite.config.ts | ||
|
|
@@ -35,6 +36,7 @@ import { | |
| formatAlreadyRunningError, | ||
| tryAcquireLockfile, | ||
| } from "./server/dev-lockfile.js"; | ||
| import { generateRouteTypes } from "./typegen.js"; | ||
|
|
||
| // ─── Resolve Vite from the project root ──────────────────────────────────────── | ||
| // | ||
|
|
@@ -721,6 +723,26 @@ async function check() { | |
| console.log(formatReport(result)); | ||
| } | ||
|
|
||
| async function typegen() { | ||
| const parsed = parseArgs(rawArgs); | ||
| if (parsed.help) return printHelp("typegen"); | ||
|
|
||
| const root = path.resolve(parsed.positionals?.[0] ?? process.cwd()); | ||
| loadDotenv({ | ||
| root, | ||
| mode: "production", | ||
| }); | ||
| const resolvedNextConfig = await resolveNextConfig( | ||
|
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: |
||
| await loadNextConfig(root, PHASE_PRODUCTION_BUILD), | ||
| root, | ||
| ); | ||
| const outputPath = await generateRouteTypes({ | ||
| root, | ||
| pageExtensions: resolvedNextConfig.pageExtensions, | ||
| }); | ||
| console.log(`\n Generated route types at ${path.relative(root, outputPath)}\n`); | ||
| } | ||
|
|
||
| async function initCommand() { | ||
| const parsed = parseArgs(rawArgs); | ||
| if (parsed.help) return printHelp("init"); | ||
|
|
@@ -889,6 +911,22 @@ function printHelp(cmd?: string) { | |
| return; | ||
| } | ||
|
|
||
| if (cmd === "typegen") { | ||
| console.log(` | ||
| vinext typegen - Generate App Router route helper types | ||
|
|
||
| Usage: vinext typegen [directory] [options] | ||
|
|
||
| Generates Next-compatible global route helpers for App Router projects: | ||
| PageProps, LayoutProps, and RouteContext. Output is written to | ||
| .next/types/routes.d.ts under the target directory. | ||
|
|
||
| Options: | ||
| -h, --help Show this help | ||
| `); | ||
| return; | ||
| } | ||
|
|
||
| if (cmd === "lint") { | ||
| console.log(` | ||
| vinext lint - Run linter | ||
|
|
@@ -914,6 +952,7 @@ function printHelp(cmd?: string) { | |
| build Build for production | ||
| start Start production server | ||
| deploy Deploy to Cloudflare Workers | ||
| typegen Generate App Router route helper types | ||
| init Migrate a Next.js project to vinext | ||
| check Scan Next.js app for compatibility | ||
| lint Run linter | ||
|
|
@@ -926,6 +965,7 @@ function printHelp(cmd?: string) { | |
| vinext dev Start dev server on port 3000 | ||
| vinext dev -p 4000 Start dev server on port 4000 | ||
| vinext build Build for production | ||
| vinext typegen Generate route helper types | ||
| vinext start Start production server | ||
| vinext deploy Deploy to Cloudflare Workers | ||
| vinext init Migrate a Next.js project | ||
|
|
@@ -992,6 +1032,13 @@ switch (command) { | |
| }); | ||
| break; | ||
|
|
||
| case "typegen": | ||
| typegen().catch((e) => { | ||
| console.error(e); | ||
| process.exit(1); | ||
| }); | ||
| break; | ||
|
|
||
| case "lint": | ||
| lint().catch((e) => { | ||
| console.error(e); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -85,6 +85,7 @@ import { createRscClientReferenceLoadersPlugin } from "./plugins/rsc-client-refe | |||||||||||||||
| import { createInstrumentationClientTransformPlugin } from "./plugins/instrumentation-client.js"; | ||||||||||||||||
| import { createOptimizeImportsPlugin } from "./plugins/optimize-imports.js"; | ||||||||||||||||
| import { createOgInlineFetchAssetsPlugin, ogAssetsPlugin } from "./plugins/og-assets.js"; | ||||||||||||||||
| import { generateRouteTypes } from "./typegen.js"; | ||||||||||||||||
| import { | ||||||||||||||||
| mergeOptimizeDepsExclude, | ||||||||||||||||
| SSR_EXTERNAL_REACT_ENTRIES, | ||||||||||||||||
|
|
@@ -627,6 +628,15 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||||||||||||
| return _generateClientEntry(pagesDir, nextConfig, fileMatcher); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| async function writeRouteTypes(): Promise<void> { | ||||||||||||||||
| if (!hasAppDir) return; | ||||||||||||||||
| await generateRouteTypes({ | ||||||||||||||||
| root, | ||||||||||||||||
| appDir, | ||||||||||||||||
| pageExtensions: nextConfig.pageExtensions, | ||||||||||||||||
| }); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Auto-register @vitejs/plugin-rsc when App Router is detected. | ||||||||||||||||
| // Check eagerly at call time using the same heuristic as config(). | ||||||||||||||||
| // Must mirror the full detection logic: check {base}/app then {base}/src/app. | ||||||||||||||||
|
|
@@ -936,6 +946,9 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||||||||||||
| instrumentationPath = findInstrumentationFile(root, fileMatcher); | ||||||||||||||||
| instrumentationClientPath = findInstrumentationClientFile(root, fileMatcher); | ||||||||||||||||
| middlewarePath = findMiddlewareFile(root, fileMatcher); | ||||||||||||||||
| if (env?.command === "build") { | ||||||||||||||||
| await writeRouteTypes(); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Merge env from next.config.js with NEXT_PUBLIC_* env vars | ||||||||||||||||
| const defines = getNextPublicEnvDefines(); | ||||||||||||||||
|
|
@@ -2177,6 +2190,42 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||||||||||||
| invalidateRootParamsModule(); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| let appRouteTypeGeneration: Promise<void> | null = null; | ||||||||||||||||
| let appRouteTypeGenerationPending = false; | ||||||||||||||||
|
|
||||||||||||||||
| function warnRouteTypeGenerationFailure(error: unknown) { | ||||||||||||||||
| server.config.logger.warn( | ||||||||||||||||
| `[vinext] Failed to regenerate route types: ${ | ||||||||||||||||
| error instanceof Error ? error.message : String(error) | ||||||||||||||||
| }`, | ||||||||||||||||
| ); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| async function drainAppRouteTypeGeneration() { | ||||||||||||||||
|
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. The drain loop design is clean. One subtle detail worth calling out: the The |
||||||||||||||||
| while (appRouteTypeGenerationPending) { | ||||||||||||||||
| appRouteTypeGenerationPending = false; | ||||||||||||||||
| try { | ||||||||||||||||
| await writeRouteTypes(); | ||||||||||||||||
| } catch (error) { | ||||||||||||||||
| warnRouteTypeGenerationFailure(error); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| function regenerateAppRouteTypes() { | ||||||||||||||||
| appRouteTypeGenerationPending = true; | ||||||||||||||||
| if (appRouteTypeGeneration) return; | ||||||||||||||||
|
|
||||||||||||||||
| appRouteTypeGeneration = drainAppRouteTypeGeneration().finally(() => { | ||||||||||||||||
| appRouteTypeGeneration = null; | ||||||||||||||||
| // A watcher event may have arrived after the drain loop's final | ||||||||||||||||
| // check but before this finally runs; restart the loop if so. | ||||||||||||||||
| if (appRouteTypeGenerationPending) regenerateAppRouteTypes(); | ||||||||||||||||
| }); | ||||||||||||||||
|
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. The
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+2219
to
+2225
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. The drain + finally re-check design is clean and correct. The comment at line 2097-2098 explaining the edge case is helpful. One observation: if
Comment on lines
+2219
to
+2225
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. The drain + finally re-check design is clean and correct. The comment explaining the edge case is helpful. Verified: if |
||||||||||||||||
|
|
||||||||||||||||
| regenerateAppRouteTypes(); | ||||||||||||||||
|
|
||||||||||||||||
| // Node throws on unhandled 'error' events on sockets. When a browser | ||||||||||||||||
| // drops the connection mid-response (common in dev: HMR triggers a | ||||||||||||||||
| // reload while an RSC stream is still flushing), the next res.write | ||||||||||||||||
|
|
@@ -2195,6 +2244,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||||||||||||
| } | ||||||||||||||||
| if (hasAppDir && shouldInvalidateAppRouteFile(appDir, filePath, fileMatcher)) { | ||||||||||||||||
| invalidateAppRoutingModules(); | ||||||||||||||||
| regenerateAppRouteTypes(); | ||||||||||||||||
|
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. Typegen regeneration hooks on However, if a user renames Just documenting the reasoning for future readers. |
||||||||||||||||
| } | ||||||||||||||||
| }); | ||||||||||||||||
| server.watcher.on("unlink", (filePath: string) => { | ||||||||||||||||
|
|
@@ -2203,6 +2253,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||||||||||||
| } | ||||||||||||||||
| if (hasAppDir && shouldInvalidateAppRouteFile(appDir, filePath, fileMatcher)) { | ||||||||||||||||
| invalidateAppRoutingModules(); | ||||||||||||||||
| regenerateAppRouteTypes(); | ||||||||||||||||
| } | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -213,6 +213,8 @@ export type RouteManifestRouteHandler = { | |
| export type RouteManifestLayout = { | ||
| id: string; | ||
| treePath: string; | ||
| patternParts: readonly string[]; | ||
| paramNames: readonly string[]; | ||
| rootBoundaryId: RootBoundaryId | null; | ||
| }; | ||
|
|
||
|
|
@@ -401,9 +403,12 @@ function createStaticSegmentGraph(routes: readonly AppRouteGraphRoute[]): Static | |
| if (existingLayout) { | ||
| assertRouteManifestRootBoundary("layout", route, layoutId, existingLayout.rootBoundaryId); | ||
| } | ||
| const layoutRouteParts = convertTreePathToRouteParts(treePath); | ||
| const layout = { | ||
| id: layoutId, | ||
| treePath, | ||
| patternParts: layoutRouteParts.urlSegments, | ||
| paramNames: layoutRouteParts.params, | ||
| rootBoundaryId: route.ids.rootBoundary, | ||
| }; | ||
| layouts.set(layoutId, layout); | ||
|
|
@@ -455,6 +460,32 @@ function createStaticSegmentGraph(routes: readonly AppRouteGraphRoute[]): Static | |
| for (const slot of route.parallelSlots) { | ||
| const ownerLayoutId = findSlotOwnerLayoutId(route, slot); | ||
| const defaultId = slot.defaultPath ? createAppRouteGraphDefaultId(slot.id) : null; | ||
| if (slot.layoutPath) { | ||
|
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. Nice — materializing slot-local layouts in the segment graph is the right call. This means One question: if a slot has a
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. The comment at lines 463-467 is well-written and explains a non-obvious invariant. This is exactly the kind of documentation that prevents future confusion. One edge case to consider for a follow-up: if a slot directory has a |
||
| // Materialize the slot-local layout as its own entry so consumers | ||
| // (e.g. typegen) can distinguish it from the owning layout. Note | ||
| // that this layout may have zero entries in `slots`: the slot | ||
| // itself is registered below against `ownerLayoutId`, which points | ||
| // to the ancestor layout that owns the slot prop. | ||
|
Comment on lines
+463
to
+468
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 comment is well-written and explains a non-obvious invariant — slot-local layouts may have zero entries in
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 documentation of a non-obvious invariant. This comment will save future readers from incorrectly assuming every layout has at least one slot entry.
Comment on lines
+463
to
+468
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 documentation of a non-obvious invariant. This comment will prevent future readers from incorrectly assuming every layout has at least one slot entry. |
||
| const slotLayoutTreePath = createSlotLayoutTreePath(slot); | ||
| const slotLayoutId = createAppRouteGraphLayoutId(slotLayoutTreePath); | ||
| const existingLayout = layouts.get(slotLayoutId); | ||
| if (existingLayout) { | ||
| assertRouteManifestRootBoundary( | ||
| "layout", | ||
| route, | ||
| slotLayoutId, | ||
| existingLayout.rootBoundaryId, | ||
| ); | ||
| } | ||
| const slotLayoutRouteParts = convertTreePathToRouteParts(slotLayoutTreePath); | ||
| layouts.set(slotLayoutId, { | ||
| id: slotLayoutId, | ||
| treePath: slotLayoutTreePath, | ||
| patternParts: slotLayoutRouteParts.urlSegments, | ||
| paramNames: slotLayoutRouteParts.params, | ||
| rootBoundaryId: route.ids.rootBoundary, | ||
| }); | ||
| } | ||
| slots.set(slot.id, { | ||
| id: slot.id, | ||
| key: slot.key, | ||
|
|
@@ -510,6 +541,12 @@ function findSlotOwnerLayoutId( | |
| return route.ids.layouts[slot.layoutIndex] ?? null; | ||
| } | ||
|
|
||
| function createSlotLayoutTreePath(slot: AppRouteGraphParallelSlot): string { | ||
| const slotSegment = `@${slot.name}`; | ||
| if (slot.ownerTreePath === "/") return `/${slotSegment}`; | ||
| return `${slot.ownerTreePath}/${slotSegment}`; | ||
| } | ||
|
|
||
| function createRouteManifestSlotBinding( | ||
| route: AppRouteGraphRoute, | ||
| slot: AppRouteGraphParallelSlot, | ||
|
|
@@ -1237,6 +1274,19 @@ function createAppRouteGraphTreePath( | |
| return `/${treePathSegments.join("/")}`; | ||
| } | ||
|
|
||
| function convertTreePathToRouteParts(treePath: string): { | ||
| urlSegments: string[]; | ||
| params: string[]; | ||
| } { | ||
| if (treePath === "/") return { urlSegments: [], params: [] }; | ||
| const segments = treePath.split("/").filter(Boolean); | ||
| const routeParts = convertSegmentsToRouteParts(segments); | ||
| if (!routeParts) { | ||
| throw new Error(`Invalid App Router layout tree path "${treePath}".`); | ||
| } | ||
| return { urlSegments: routeParts.urlSegments, params: routeParts.params }; | ||
| } | ||
|
|
||
| /** | ||
| * Compute the tree position (directory depth from app root) for each layout. | ||
| * Root layout = 0, a layout at app/blog/ = 1, app/blog/(group)/ = 2. | ||
|
|
@@ -1882,7 +1932,7 @@ function collectInterceptingPages( | |
| * Used by computeInterceptTarget, convertSegmentsToRouteParts, and | ||
| * hasRemainingVisibleSegments — keep this the single source of truth. | ||
| */ | ||
| function isInvisibleSegment(segment: string): boolean { | ||
| export function isInvisibleSegment(segment: string): boolean { | ||
|
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 — exporting |
||
| if (segment === ".") return true; | ||
| if (segment.startsWith("(") && segment.endsWith(")")) return true; | ||
| if (segment.startsWith("@")) return true; | ||
|
|
||
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.
how come these changes were needed?
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.
This was needed for the Windows create-next-app smoke test harness, not for route typegen behavior. The assertion stays the same: the freshly scaffolded app must serve a real HTTP 200 from the vinext dev server. In the failing run, Vite had already started on 127.0.0.1:3099 and RSC connected, but the curl probe returned 000 for all attempts. In the passing run with the Node probe, the first completed request returned
GET / 200 in 2.7s, including 2.1s of cold compile time, and the probe observed 200 on attempt 3. So this does not lower the check or bypass an app failure; it keeps the same HTTP 200 requirement while avoiding a Windows/Git Bash curl readiness false negative during cold compile. I can split this CI stabilization into a separate PR if you prefer to keep this route typegen PR scoped only to the typegen changes.