-
Notifications
You must be signed in to change notification settings - Fork 276
feat: add optional dev-time next typegen
#651
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
base: main
Are you sure you want to change the base?
Changes from all commits
2849219
056b6f6
436bad6
ad3b936
9c8b1a6
13e693b
7937f92
79b276c
a2f061d
0fbb66e
da51df8
c0dc029
f38af9d
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,139 @@ | ||||||||||||||
| import { spawn, type ChildProcess } from "node:child_process"; | ||||||||||||||
| import { createRequire } from "node:module"; | ||||||||||||||
| import path from "node:path"; | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Temporary bridge for route-aware type generation. | ||||||||||||||
| * | ||||||||||||||
| * Vinext should eventually generate these types from its own route tree | ||||||||||||||
| * instead of delegating to `next typegen`, but this improves dev-time DX in | ||||||||||||||
| * the meantime without making Next.js a hard requirement for startup. | ||||||||||||||
| * | ||||||||||||||
| * Tracking issue: https://github.com/cloudflare/vinext/issues/664 | ||||||||||||||
| */ | ||||||||||||||
| export interface NextTypegenControllerOptions { | ||||||||||||||
| root: string; | ||||||||||||||
| enabled?: boolean; | ||||||||||||||
| debounceMs?: number; | ||||||||||||||
| logger?: Pick<Console, "info" | "warn">; | ||||||||||||||
| resolveNextBin?: (root: string) => string | null; | ||||||||||||||
| spawnImpl?: typeof spawn; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| export interface NextTypegenController { | ||||||||||||||
| start(): void; | ||||||||||||||
| schedule(): void; | ||||||||||||||
| close(): void; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| export function resolveNextTypegenBin(root: string): string | null { | ||||||||||||||
| try { | ||||||||||||||
| const projectRequire = createRequire(path.join(root, "package.json")); | ||||||||||||||
| return projectRequire.resolve("next/dist/bin/next"); | ||||||||||||||
| } catch { | ||||||||||||||
| return null; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| export function createNextTypegenController( | ||||||||||||||
| options: NextTypegenControllerOptions, | ||||||||||||||
| ): NextTypegenController { | ||||||||||||||
| const { | ||||||||||||||
| root, | ||||||||||||||
| enabled = true, | ||||||||||||||
| debounceMs = 150, | ||||||||||||||
| logger = console, | ||||||||||||||
| resolveNextBin = resolveNextTypegenBin, | ||||||||||||||
| spawnImpl = spawn, | ||||||||||||||
| } = options; | ||||||||||||||
|
|
||||||||||||||
| let timer: ReturnType<typeof setTimeout> | null = null; | ||||||||||||||
| let child: ChildProcess | null = null; | ||||||||||||||
| let pending = false; | ||||||||||||||
| let nextBin: string | null | undefined; | ||||||||||||||
| let missingNextLogged = false; | ||||||||||||||
| let firstSuccessLogged = false; | ||||||||||||||
| let closed = false; | ||||||||||||||
|
|
||||||||||||||
| function getNextBin(): string | null { | ||||||||||||||
| if (nextBin !== undefined) return nextBin; | ||||||||||||||
| nextBin = resolveNextBin(root); | ||||||||||||||
| if (!nextBin && !missingNextLogged) { | ||||||||||||||
| missingNextLogged = true; | ||||||||||||||
| logger.info("[vinext] Skipping dev typegen: `next` is not installed in this project."); | ||||||||||||||
| } | ||||||||||||||
| return nextBin; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| function run(): void { | ||||||||||||||
| if (!enabled || closed) return; | ||||||||||||||
|
|
||||||||||||||
| const resolvedNextBin = getNextBin(); | ||||||||||||||
| if (!resolvedNextBin) return; | ||||||||||||||
|
|
||||||||||||||
| if (child) { | ||||||||||||||
| pending = true; | ||||||||||||||
| return; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| child = spawnImpl(process.execPath, [resolvedNextBin, "typegen"], { | ||||||||||||||
|
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. Passing Consider either removing the explicit
Suggested change
|
||||||||||||||
| cwd: root, | ||||||||||||||
| stdio: "ignore", | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| child.once("error", (error) => { | ||||||||||||||
| logger.warn(`[vinext] Failed to run \`next typegen\`: ${error.message}`); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| child.once("exit", (code, signal) => { | ||||||||||||||
| child = null; | ||||||||||||||
|
|
||||||||||||||
| if (closed) return; | ||||||||||||||
|
|
||||||||||||||
| if (code !== 0) { | ||||||||||||||
|
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.
Consider adding a if (code === 0 && !firstRunLogged) {
firstRunLogged = true;
logger.info("[vinext] `next typegen` completed.");
} |
||||||||||||||
| const detail = signal ? `signal ${signal}` : `exit code ${code ?? "unknown"}`; | ||||||||||||||
| logger.warn(`[vinext] \`next typegen\` exited with ${detail}.`); | ||||||||||||||
| } else if (!firstSuccessLogged) { | ||||||||||||||
| firstSuccessLogged = true; | ||||||||||||||
| logger.info("[vinext] `next typegen` completed."); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if (pending) { | ||||||||||||||
| pending = false; | ||||||||||||||
| run(); | ||||||||||||||
| } | ||||||||||||||
| }); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| function scheduleWithDelay(delayMs: number): void { | ||||||||||||||
| if (!enabled || closed) return; | ||||||||||||||
| if (timer) clearTimeout(timer); | ||||||||||||||
| timer = setTimeout(() => { | ||||||||||||||
| timer = null; | ||||||||||||||
| run(); | ||||||||||||||
| }, delayMs); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return { | ||||||||||||||
| start() { | ||||||||||||||
|
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.
Suggested change
|
||||||||||||||
| // Defer the first run so configureServer can finish without waiting for | ||||||||||||||
| // `next typegen` to complete synchronously. | ||||||||||||||
| scheduleWithDelay(0); | ||||||||||||||
| }, | ||||||||||||||
| schedule() { | ||||||||||||||
| scheduleWithDelay(debounceMs); | ||||||||||||||
| }, | ||||||||||||||
| close() { | ||||||||||||||
| closed = true; | ||||||||||||||
| if (timer) { | ||||||||||||||
| clearTimeout(timer); | ||||||||||||||
| timer = null; | ||||||||||||||
| } | ||||||||||||||
| if (child) { | ||||||||||||||
|
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.
Also, after
Suggested change
|
||||||||||||||
| child.removeAllListeners(); | ||||||||||||||
| child.kill(); | ||||||||||||||
| child = null; | ||||||||||||||
| } | ||||||||||||||
| }, | ||||||||||||||
| }; | ||||||||||||||
| } | ||||||||||||||
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.
We should avoid have having a hard dependency/requirement on Next.js for Vinext to be usable
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.
That makes sense. I agree Vinext itself should not depend on Next.js to be usable.
In this PR, my intent was only to add an optional dev-time integration: if
nextis installed in the project already, Vinext will opportunistically runnext typegen; if it is not installed, startup still succeeds and we skip it.For now, this is mainly intended as a pragmatic way to improve the developer experience in the short term. We can still follow up later with a native Vinext typegen implementation so this no longer depends on Next.js at all.