-
Notifications
You must be signed in to change notification settings - Fork 105
fix: normalize workbench tests #292
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
87ce599
e532869
cd1888b
ce120f2
67fb677
2563978
ea58dc1
63d92ce
a59f764
63a9c4c
761278f
f30c295
4959f80
6f92685
f94a38b
e2ea115
482d0da
b181a03
353d877
06b916d
12a524d
00f3e51
4792d39
ef98862
47e7bfb
a38c472
6b5d113
b6d2d7c
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,5 @@ | ||
| --- | ||
| "@workflow/nitro": patch | ||
| --- | ||
|
|
||
| Add Vite middleware to handle 404s in workflow routes from Nitro and silence undefined unhandled rejections |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@workflow/sveltekit": patch | ||
| --- | ||
|
|
||
| Fix SvelteKit plugin reading deleted files on HMR |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,13 @@ | ||
| import type { Nitro } from 'nitro/types'; | ||
| import type { Plugin } from 'vite'; | ||
| import type { HotUpdateOptions, Plugin } from 'vite'; | ||
| import { LocalBuilder } from './builders.js'; | ||
| import type { ModuleOptions } from './index.js'; | ||
| import nitroModule from './index.js'; | ||
| import { workflowRollupPlugin } from './rollup.js'; | ||
|
|
||
| export function workflow(options?: ModuleOptions): Plugin[] { | ||
| let builder: LocalBuilder | undefined; | ||
|
|
||
| return [ | ||
| workflowRollupPlugin(), | ||
| { | ||
|
|
@@ -18,9 +21,96 @@ export function workflow(options?: ModuleOptions): Plugin[] { | |
| ...options, | ||
| _vite: true, | ||
| }; | ||
| if (nitro.options.dev) { | ||
| builder = new LocalBuilder(nitro); | ||
| } | ||
| return nitroModule.setup(nitro); | ||
| }, | ||
| }, | ||
| // 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. | ||
| configureServer(server) { | ||
| // Add middleware to intercept 404s on workflow routes before Vite's SPA fallback | ||
| return () => { | ||
| server.middlewares.use((req, res, next) => { | ||
| // Only handle workflow webhook routes | ||
| if (!req.url?.startsWith('/.well-known/workflow/v1/')) { | ||
| return next(); | ||
| } | ||
|
|
||
| // Wrap writeHead to ensure we send empty body for 404s | ||
| const originalWriteHead = res.writeHead; | ||
| res.writeHead = function (this: typeof res, ...args: any[]) { | ||
| 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. | ||
| // 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 | ||
| res.setHeader('Content-Length', '0'); | ||
adriandlam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // @ts-expect-error - Complex overload signature | ||
| return originalWriteHead.apply(this, args); | ||
| } as any; | ||
|
|
||
| next(); | ||
| }); | ||
| }; | ||
| }, | ||
| // TODO: Move this to @workflow/vite or something since this is vite specific | ||
|
Collaborator
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. You can start this by extracting it to a new function. Vite plugin can return an array (so reusable parts can be shared between nitro/svelete) |
||
| async hotUpdate(options: HotUpdateOptions) { | ||
| const { file, server, read } = options; | ||
|
|
||
| // Check if this is a TS/JS file that might contain workflow directives | ||
| const jsTsRegex = /\.(ts|tsx|js|jsx|mjs|cjs)$/; | ||
| if (!jsTsRegex.test(file)) { | ||
| return; | ||
| } | ||
|
|
||
| // Read the file to check for workflow/step directives | ||
| let content: string; | ||
| try { | ||
| 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: '*', | ||
|
Collaborator
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. hmm too aggressive?
Collaborator
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. probably fine for now and we can revisit |
||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const useWorkflowPattern = /^\s*(['"])use workflow\1;?\s*$/m; | ||
| const useStepPattern = /^\s*(['"])use step\1;?\s*$/m; | ||
|
|
||
| if ( | ||
| !useWorkflowPattern.test(content) && | ||
| !useStepPattern.test(content) | ||
| ) { | ||
| 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: '*', | ||
| }); | ||
|
|
||
| // Let Vite handle the normal HMR for the changed file | ||
| return; | ||
| }, | ||
| }, | ||
| ]; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| import { Hono } from 'hono'; | ||
| import { getHookByToken, getRun, resumeHook, start } from 'workflow/api'; | ||
| import { hydrateWorkflowArguments } from 'workflow/internal/serialization'; | ||
| import { allWorkflows } from './_workflows.js'; | ||
| import { | ||
| WorkflowRunFailedError, | ||
| WorkflowRunNotCompletedError, | ||
| } from 'workflow/internal/errors'; | ||
| import { hydrateWorkflowArguments } from 'workflow/internal/serialization'; | ||
| import { allWorkflows } from './_workflows.js'; | ||
|
|
||
| const app = new Hono(); | ||
|
|
||
|
|
@@ -163,8 +163,9 @@ app.post('/api/hook', async ({ req }) => { | |
| } catch (error) { | ||
| console.log('error during getHookByToken', error); | ||
| // TODO: `WorkflowAPIError` is not exported, so for now | ||
|
Collaborator
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. cc @TooTallNate we should have a |
||
| // we'll return 404 assuming it's the "invalid" token test case | ||
| return Response.json(null, { status: 404 }); | ||
| // we'll return 422 assuming it's the "invalid" token test case | ||
adriandlam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // NOTE: Need to return 422 because Nitro passes 404 requests to the dev server to handle. | ||
| return Response.json(null, { status: 422 }); | ||
| } | ||
|
|
||
| await resumeHook(hook.token, { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // NOTE: This route isn't needed/ever used, we're just | ||
| // using it because webpack relies on esbuild's tree shaking | ||
|
|
||
| import { start } from 'workflow/api'; | ||
| import { addTenWorkflow } from '@/workflows/98_duplicate_case'; | ||
|
|
||
| export async function GET(_: Request) { | ||
| const run = await start(addTenWorkflow, [10]); | ||
| const result = await run.returnValue; | ||
| return Response.json({ result }); | ||
| } |
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.
crazy
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.
cc @pi0 you know about this?