-
Notifications
You must be signed in to change notification settings - Fork 354
feat(dev): detect 'use cache' module-scope deadlocks early in dev (#1126) #1157
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
1cccd4b
8ab447b
e7cc4b4
538e3ad
9e9624a
35f0da7
d888012
9bb642e
11705f4
abb5c21
5cf00c6
e2f69c8
9bb6753
00f736c
066cd32
f82eacd
e9cd15e
b408fb2
299ce73
a29995d
6ac4309
ec0c359
b1b6873
5eefe5f
6ad24aa
cedafca
0344a49
23f94c7
c9cebc3
422378a
5385eab
2209579
fa25fea
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 | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -344,6 +344,19 @@ function hasServerOnlyMarkerImport(code: string): boolean { | |||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const __dirname = import.meta.dirname; | ||||||||||||||||
|
|
||||||||||||||||
| // Lazy load the use-cache probe pool so it is only loaded when the dev | ||||||||||||||||
| // server starts (configureServer), not during production builds. | ||||||||||||||||
| let _probePoolModulePromise: Promise<typeof import("./server/use-cache-probe-pool.js")> | null = | ||||||||||||||||
| null; | ||||||||||||||||
| function getProbePoolModule(): Promise<typeof import("./server/use-cache-probe-pool.js")> { | ||||||||||||||||
| _probePoolModulePromise ??= import("./server/use-cache-probe-pool.js").catch((error) => { | ||||||||||||||||
| _probePoolModulePromise = null; | ||||||||||||||||
| throw error; | ||||||||||||||||
| }); | ||||||||||||||||
| return _probePoolModulePromise; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| type VitePluginReactModule = typeof import("@vitejs/plugin-react"); | ||||||||||||||||
|
|
||||||||||||||||
| function resolveOptionalDependency(projectRoot: string, specifier: string): string | null { | ||||||||||||||||
|
|
@@ -2133,6 +2146,9 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||||||||||||
| defines["process.env.__VINEXT_DEPLOYMENT_ID"] = JSON.stringify( | ||||||||||||||||
| nextConfig.deploymentId ?? "", | ||||||||||||||||
| ); | ||||||||||||||||
| defines["process.env.__VINEXT_USE_CACHE_TIMEOUT_MS"] = JSON.stringify( | ||||||||||||||||
| nextConfig.useCacheTimeout * 1_000, | ||||||||||||||||
| ); | ||||||||||||||||
| // Public `process.env.NEXT_DEPLOYMENT_ID` — Next.js statically inlines | ||||||||||||||||
| // this into client (and web worker) bundles via its DefinePlugin so | ||||||||||||||||
| // that user code like `new Worker(new URL('./w.ts', import.meta.url))` | ||||||||||||||||
|
|
@@ -4099,7 +4115,7 @@ export const loadServerActionClient = ${ | |||||||||||||||
| }, | ||||||||||||||||
| }, | ||||||||||||||||
|
|
||||||||||||||||
| configureServer(server: ViteDevServer) { | ||||||||||||||||
| async configureServer(server: ViteDevServer) { | ||||||||||||||||
| // Watch route files for additions/removals to invalidate route cache. | ||||||||||||||||
| const pageExtensions = fileMatcher.extensionRegex; | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -4194,11 +4210,30 @@ export const loadServerActionClient = ${ | |||||||||||||||
| pagesRunner?.clearCache(); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| function invalidateAppRoutingModules() { | ||||||||||||||||
| async function invalidateAppRoutingModules() { | ||||||||||||||||
| invalidateAppRouteCache(); | ||||||||||||||||
| invalidateMetadataFileCache(); | ||||||||||||||||
| invalidateRscEntryModule(); | ||||||||||||||||
| invalidateRootParamsModule(); | ||||||||||||||||
| await resetUseCacheProbePool(); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| async function resetUseCacheProbePool() { | ||||||||||||||||
| const { tearDownUseCacheProbePool, initUseCacheProbePool } = await getProbePoolModule(); | ||||||||||||||||
|
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 One subtlety: if Non-blocking.
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. Non-blocking: One subtlety worth noting for future maintainers: if
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: making This is fine since the call sites do log the error, but the behavior change is worth being aware of — previously a throw from |
||||||||||||||||
| // Tear down the use-cache probe pool so the next probe starts with | ||||||||||||||||
| // fresh code after HMR invalidation. | ||||||||||||||||
| tearDownUseCacheProbePool(); | ||||||||||||||||
|
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. Critical: probe permanently disabled after first HMR. Either re-initialize here:
Suggested change
Or restructure
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 teardown+reinit pattern looks correct now. One thing to watch: if |
||||||||||||||||
| // Re-initialize so probes continue working after HMR. | ||||||||||||||||
| const rscEnv = server.environments["rsc"]; | ||||||||||||||||
| if (rscEnv) { | ||||||||||||||||
| initUseCacheProbePool(rscEnv); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+4213
to
+4230
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. One thing to watch: if This is actually fine — the captured Non-blocking — the retry-on-error pattern in |
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| function tearDownUseCacheProbePoolOnServerClose() { | ||||||||||||||||
| void getProbePoolModule().then(({ tearDownUseCacheProbePool }) => { | ||||||||||||||||
| tearDownUseCacheProbePool(); | ||||||||||||||||
| }); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| let hybridRouteValidation: Promise<void> = Promise.resolve(); | ||||||||||||||||
|
|
@@ -4341,7 +4376,7 @@ export const loadServerActionClient = ${ | |||||||||||||||
| routeChanged = true; | ||||||||||||||||
| } | ||||||||||||||||
| if (hasAppDir && shouldInvalidateAppRouteFile(appDir, filePath, fileMatcher)) { | ||||||||||||||||
| invalidateAppRoutingModules(); | ||||||||||||||||
| void invalidateAppRoutingModules(); | ||||||||||||||||
| regenerateAppRouteTypes(); | ||||||||||||||||
| routeChanged = true; | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -4382,7 +4417,7 @@ export const loadServerActionClient = ${ | |||||||||||||||
| routeChanged = true; | ||||||||||||||||
| } | ||||||||||||||||
| if (hasAppDir && shouldInvalidateAppRouteFile(appDir, filePath, fileMatcher)) { | ||||||||||||||||
| invalidateAppRoutingModules(); | ||||||||||||||||
| void invalidateAppRoutingModules(); | ||||||||||||||||
| regenerateAppRouteTypes(); | ||||||||||||||||
| routeChanged = true; | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -4393,6 +4428,17 @@ export const loadServerActionClient = ${ | |||||||||||||||
| revalidateHybridRoutes(); | ||||||||||||||||
| } | ||||||||||||||||
| }); | ||||||||||||||||
| server.watcher.on("change", (filePath: string) => { | ||||||||||||||||
| if ( | ||||||||||||||||
| hasAppDir && | ||||||||||||||||
| isInsideDirectory(appDir, filePath) && | ||||||||||||||||
| fileMatcher.extensionRegex.test(filePath) | ||||||||||||||||
| ) { | ||||||||||||||||
| void resetUseCacheProbePool(); | ||||||||||||||||
| } | ||||||||||||||||
| }); | ||||||||||||||||
| server.watcher.once("close", tearDownUseCacheProbePoolOnServerClose); | ||||||||||||||||
| server.httpServer?.once("close", tearDownUseCacheProbePoolOnServerClose); | ||||||||||||||||
|
|
||||||||||||||||
| // ── Dev request origin check ───────────────────────────────────── | ||||||||||||||||
| // Registered directly (not in the returned function) so it runs | ||||||||||||||||
|
|
@@ -4527,6 +4573,18 @@ export const loadServerActionClient = ${ | |||||||||||||||
| // it is flushed to the client. | ||||||||||||||||
| // 2. Logs the full request after res finishes, using those timings. | ||||||||||||||||
| if (hasAppDir) { | ||||||||||||||||
| // Initialize the "use cache" deadlock probe pool for the App | ||||||||||||||||
| // Router dev server. We bind to the RSC environment because | ||||||||||||||||
| // "use cache" functions run inside the RSC module graph. | ||||||||||||||||
| const rscEnv = server.environments["rsc"]; | ||||||||||||||||
| if (rscEnv) { | ||||||||||||||||
| getProbePoolModule() | ||||||||||||||||
|
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: this is fire-and-forget, so the first request arriving before the probe pool finishes initializing will silently skip deadlock detection. This is fine for a dev-only diagnostic — just noting it for awareness. The error logging in |
||||||||||||||||
| .then(({ initUseCacheProbePool }) => initUseCacheProbePool(rscEnv)) | ||||||||||||||||
| .catch((err) => { | ||||||||||||||||
| console.warn("[vinext] Failed to initialize use-cache probe pool:", err); | ||||||||||||||||
| }); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| server.middlewares.use((req, res, next) => { | ||||||||||||||||
| const url = req.url ?? "/"; | ||||||||||||||||
| // Skip Vite internals, HMR, and static assets. | ||||||||||||||||
|
|
||||||||||||||||
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.
Latency on HMR invalidation.
invalidateAppRoutingModulesis now async because itawaitsgetProbePoolModule(). The three synchronous invalidation calls above (lines 2064-2066) run immediately, but the probe teardown+reinit is deferred to the next microtask. This means there's a brief window where routes are invalidated but the old probe closure is still active.In practice this is fine — the old probe closure captured
envlocally and will still work correctly if it fires during this window. Just noting the ordering for future readers.