-
Notifications
You must be signed in to change notification settings - Fork 135
fix: nitro build race conditions #444
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
cc5b4bf
4f0531d
1502847
ae8bc8d
cb3a67a
4388e2b
9746d98
a39dce9
d285500
33d95dc
69b8a16
f3407ee
9f82337
6d983bd
9ff3948
55d66c7
7157ec9
ddc720a
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,7 @@ | ||
| --- | ||
| "@workflow/sveltekit": patch | ||
| "@workflow/builders": patch | ||
| "@workflow/nitro": patch | ||
| --- | ||
|
|
||
| Fix Nitro and SvelteKit build race conditions and make writing debug file atomic |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /** | ||
| * Creates a build queue that serializes async tasks to prevent race conditions. | ||
| * | ||
| * When rapid file changes trigger multiple builds, this ensures they run | ||
| * sequentially rather than concurrently, avoiding issues like: | ||
| * - Partial file reads during writes | ||
| * - Corrupted merge operations | ||
| * - Duplicate/conflicting builds | ||
| * | ||
| * @example | ||
| * const enqueue = createBuildQueue(); | ||
| * | ||
| * // These will run sequentially, not concurrently | ||
| * enqueue(() => builder.build()); | ||
| * enqueue(() => builder.build()); | ||
| */ | ||
| export function createBuildQueue() { | ||
| let queue = Promise.resolve(); | ||
|
|
||
| return (task: () => Promise<void>): Promise<void> => { | ||
| queue = queue.then(task).catch((err) => { | ||
| console.error(err); | ||
| }); | ||
| return queue; | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,11 @@ import type { Plugin as VitePlugin } from 'vite'; | |
| import type { ModuleOptions } from './index.js'; | ||
| import nitroModule from './index.js'; | ||
| import { workflowTransformPlugin } from '@workflow/rollup'; | ||
| import { createBuildQueue } from '@workflow/builders'; | ||
|
|
||
| export function workflow(options?: ModuleOptions): Plugin[] { | ||
| let builder: LocalBuilder | undefined; | ||
| let builder: LocalBuilder; | ||
|
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 View Details📝 Patch Detailsdiff --git a/packages/nitro/src/vite.ts b/packages/nitro/src/vite.ts
index 608e5fc..0437280 100644
--- a/packages/nitro/src/vite.ts
+++ b/packages/nitro/src/vite.ts
@@ -8,7 +8,7 @@ import nitroModule from './index.js';
import { workflowRollupPlugin } from './rollup.js';
export function workflow(options?: ModuleOptions): Plugin[] {
- let builder: LocalBuilder;
+ let builder: LocalBuilder | undefined;
// Build queue to serialize builds and prevent race conditions
// when rapid file changes trigger concurrent hotUpdate calls.
@@ -87,7 +87,9 @@ export function workflow(options?: ModuleOptions): Plugin[] {
content = await read();
} catch {
// File might have been deleted - trigger rebuild to update generated routes
- await enqueue(() => builder.build());
+ if (builder) {
+ await enqueue(() => builder.build());
+ }
return;
}
@@ -102,7 +104,9 @@ export function workflow(options?: ModuleOptions): Plugin[] {
}
console.log('Workflow file changed, rebuilding...');
- await enqueue(() => builder.build());
+ if (builder) {
+ await enqueue(() => builder.build());
+ }
// Let Vite handle the normal HMR for the changed file
return;
},
AnalysisUninitialized builder variable called unconditionally in hotUpdate hookWhat fails: The How to reproduce: Configure a Vite + Nitro setup where Result: Runtime error - "Workflow build failed: TypeError: Cannot read properties of undefined (reading 'build')". The error is caught and logged by the enqueue function's catch handler, but the file change is not properly processed. Expected: The Note: This issue was introduced in commit 30eb016 which refactored the code to remove previous defensive null checks (
Member
Author
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. i don't think this is true because if we're in dev then This is only true if somehow |
||
| const enqueue = createBuildQueue(); | ||
|
|
||
| return [ | ||
| workflowTransformPlugin() as VitePlugin, | ||
|
|
@@ -28,7 +30,7 @@ export function workflow(options?: ModuleOptions): Plugin[] { | |
| }, | ||
| }, | ||
| // NOTE: This is a workaround because Nitro passes the 404 requests to the dev server to handle. | ||
| // For workflow routes, we override to send an empty body to prevent Hono/Vite's SPA fallback. | ||
| // For workflow routes, we override to send an empty body to prevent Hono/Vite's SPA fallback. | ||
| configureServer(server) { | ||
| // Add middleware to intercept 404s on workflow routes before Vite's SPA fallback | ||
| return () => { | ||
|
|
@@ -44,7 +46,7 @@ export function workflow(options?: ModuleOptions): Plugin[] { | |
| const statusCode = typeof args[0] === 'number' ? args[0] : 200; | ||
|
|
||
| // NOTE: Workaround because Nitro passes 404 requests to the vite to handle. | ||
| // Causes `webhook route with invalid token` test to fail. | ||
| // Causes `webhook route with invalid token` test to fail. | ||
| // For 404s on workflow routes, ensure we're sending the right headers | ||
| if (statusCode === 404) { | ||
| // Set content-length to 0 to prevent Vite from overriding | ||
|
|
@@ -61,7 +63,7 @@ export function workflow(options?: ModuleOptions): Plugin[] { | |
| }, | ||
| // TODO: Move this to @workflow/vite or something since this is vite specific | ||
| async hotUpdate(options: HotUpdateOptions) { | ||
| const { file, server, read } = options; | ||
| const { file, read } = options; | ||
|
|
||
| // Check if this is a TS/JS file that might contain workflow directives | ||
| const jsTsRegex = /\.(ts|tsx|js|jsx|mjs|cjs)$/; | ||
|
|
@@ -75,15 +77,8 @@ export function workflow(options?: ModuleOptions): Plugin[] { | |
| content = await read(); | ||
| } catch { | ||
| // File might have been deleted - trigger rebuild to update generated routes | ||
| console.log('Workflow file deleted, rebuilding...'); | ||
| if (builder) { | ||
| await builder.build(); | ||
| } | ||
| // NOTE: Might be too aggressive | ||
| server.ws.send({ | ||
| type: 'full-reload', | ||
| path: '*', | ||
| }); | ||
| console.log('Workflow file changed, rebuilding...'); | ||
| await enqueue(() => builder.build()); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -97,17 +92,8 @@ export function workflow(options?: ModuleOptions): Plugin[] { | |
| return; | ||
| } | ||
|
|
||
| // Trigger full reload - this will cause Nitro's dev:reload hook to fire, | ||
| // which will rebuild workflows and update routes | ||
| console.log('Workflow file changed, rebuilding...'); | ||
| if (builder) { | ||
| await builder.build(); | ||
| } | ||
| server.ws.send({ | ||
| type: 'full-reload', | ||
| path: '*', | ||
| }); | ||
|
|
||
| await enqueue(() => builder.build()); | ||
| // Let Vite handle the normal HMR for the changed file | ||
| return; | ||
| }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.