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
32 changes: 26 additions & 6 deletions .github/workflows/ci.yml
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

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.

Original file line number Diff line number Diff line change
Expand Up @@ -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 &
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 improvement — explicit --host 127.0.0.1 avoids IPv6/IPv4 mismatches in CI, --max-time 2 prevents curl from hanging indefinitely on a stuck server, and the echo on each attempt makes debugging CI failures much easier. Solid reliability fix.

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 reliability fix — explicit 127.0.0.1, --max-time 2, and per-attempt logging. This should eliminate flaky CI from IPv6/IPv4 mismatches and hanging curl.

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:
Expand Down
9 changes: 9 additions & 0 deletions packages/vinext/src/cli-args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type ParsedArgs = {
prerenderAll?: boolean;
prerenderConcurrency?: number;
precompress?: boolean;
positionals?: string[];
};

// Matches long flags (--foo) and single-letter short flags (-x).
Expand Down Expand Up @@ -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];

Expand Down Expand Up @@ -166,6 +172,9 @@ export function parseArgs(args: string[]): ParsedArgs {
);
break;
}
if (!FLAG_PATTERN.test(arg)) {
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 guard correctly skips flag-looking strings (so --unknown isn't collected as a positional), but it also means unknown flags are silently swallowed. For typegen this is fine — the only expected positional is the directory — but as more commands adopt positionals, a warning for unrecognized flags would improve DX.

Not blocking for this PR.

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: unknown flags like --foobar that don't match any case and pass the FLAG_PATTERN test are silently dropped (they fall through default without being collected as positionals, which is correct, but also without any warning). This is pre-existing behavior, but since typegen is the first command that actually relies on positionals, it might be worth logging a warning for unrecognized flags in a follow-up.

addPositional(arg);
}
break;
}
}
Expand Down
47 changes: 47 additions & 0 deletions packages/vinext/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -35,6 +36,7 @@ import {
formatAlreadyRunningError,
tryAcquireLockfile,
} from "./server/dev-lockfile.js";
import { generateRouteTypes } from "./typegen.js";

// ─── Resolve Vite from the project root ────────────────────────────────────────
//
Expand Down Expand Up @@ -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(
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: resolvedNextConfig loads both loadNextConfig and resolveNextConfig, which reads the config file and resolves paths. If the project has no next.config.{js,ts,mjs}, does loadNextConfig throw or return defaults? If it throws, the error message won't be very helpful for a typegen command. Consider catching and providing a friendlier message like "No next.config found; using defaults".

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");
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
51 changes: 51 additions & 0 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
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.

The drain loop design is clean. One subtle detail worth calling out: the while loop clears pending at the top, then awaits the write. If an event arrives during the write, pending gets set to true by regenerateAppRouteTypes, and the loop runs another iteration — which is correct.

The finally block at line 2095 covers the edge case where an event arrives after the loop exits but before finally runs (i.e. during microtask scheduling of the .finally callback). Since both the event handler and finally run on the same microtask queue, this window is extremely narrow, but the guard is still correct to have. Good attention to detail.

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();
});
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.

The finally block sets appRouteTypeGeneration = null then re-checks appRouteTypeGenerationPending. Since JS is single-threaded, there's no actual race here, but the pattern is subtle. A brief comment explaining why the re-check is needed would help:

Suggested change
});
appRouteTypeGeneration = drainAppRouteTypeGeneration().finally(() => {
appRouteTypeGeneration = null;
// An event may have arrived after drain finished but before we cleared
// the in-flight promise. Re-check and restart if so.
if (appRouteTypeGenerationPending) regenerateAppRouteTypes();
});

}
Comment on lines +2219 to +2225
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.

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 writeRouteTypes() throws on every attempt (e.g. persistent permissions error), the drain loop exits after one iteration (because pending was cleared at line 2082 before the try), the finally callback fires, and no further retries happen unless a new watcher event arrives. This is the right behavior — avoid infinite retry loops on persistent errors.

Comment on lines +2219 to +2225
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.

The drain + finally re-check design is clean and correct. The comment explaining the edge case is helpful. Verified: if writeRouteTypes() throws persistently, the drain loop exits after one iteration and no infinite retry occurs — new watcher events are needed to restart. This is the right behavior.


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
Expand All @@ -2195,6 +2244,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
}
if (hasAppDir && shouldInvalidateAppRouteFile(appDir, filePath, fileMatcher)) {
invalidateAppRoutingModules();
regenerateAppRouteTypes();
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.

Typegen regeneration hooks on add and unlink but not change. This matches the existing invalidateAppRoutingModules pattern, so it's consistent.

However, if a user renames page.tsx to route.ts in the same directory (editor "save as" rather than move), the watcher fires unlink for the old file + add for the new one, so that case is covered. The gap is an in-place edit that changes exports (e.g. adding/removing generateStaticParams) — but since typegen only cares about route presence, not exports, this is fine.

Just documenting the reasoning for future readers.

}
});
server.watcher.on("unlink", (filePath: string) => {
Expand All @@ -2203,6 +2253,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
}
if (hasAppDir && shouldInvalidateAppRouteFile(appDir, filePath, fileMatcher)) {
invalidateAppRoutingModules();
regenerateAppRouteTypes();
}
});

Expand Down
52 changes: 51 additions & 1 deletion packages/vinext/src/routing/app-route-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ export type RouteManifestRouteHandler = {
export type RouteManifestLayout = {
id: string;
treePath: string;
patternParts: readonly string[];
paramNames: readonly string[];
rootBoundaryId: RootBoundaryId | null;
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
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.

Nice — materializing slot-local layouts in the segment graph is the right call. This means app/@modal/layout.tsx gets its own layout entry with a treePath of /@modal, which keeps it distinct from the root layout in typegen's LayoutRoute union.

One question: if a slot has a layoutPath but its ownerLayoutId points to an ancestor, the slot-local layout gets registered here but the slot itself (line 465) still references the ancestor via ownerLayoutId. This is correct for typegen (slot props go to the ancestor), but could it confuse other consumers of the segment graph that iterate layouts and expect every layout to have at least one slot? Worth a brief comment documenting that slot-local layouts may have zero slots in the manifest.

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.

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 layout.tsx and nested sub-routes with their own slots, those nested slots would have ownerLayoutId pointing to the slot-local layout. The current code handles this correctly because the materialized slot-local layout gets its own entry in layouts, so layoutRouteKeyForSlot in typegen can resolve it. Just confirming the chain works end-to-end.

// 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
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 comment is well-written and explains a non-obvious invariant — slot-local layouts may have zero entries in slots. Good documentation.

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 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
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 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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
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 — exporting isInvisibleSegment here and importing it in typegen.ts eliminates the prior duplication. Single source of truth.

if (segment === ".") return true;
if (segment.startsWith("(") && segment.endsWith(")")) return true;
if (segment.startsWith("@")) return true;
Expand Down
Loading
Loading