Normalize Workbenches #283
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 39bc2f4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| function generateRegistry() { | ||
| // Check if workflows directory exists | ||
| if (!fs.existsSync(workflowsDir)) { | ||
| console.error(`Error: Workflows directory not found: ${workflowsDir}`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Read all files from workflows directory | ||
| const files = fs | ||
| .readdirSync(workflowsDir) | ||
| .filter((file) => { | ||
| // Only .ts files | ||
| if (!file.endsWith('.ts')) return false; | ||
| // Skip helpers and files starting with _ | ||
| if (SKIP_FILES.includes(file)) return false; | ||
| if (file.startsWith(SKIP_PREFIX)) return false; | ||
| return true; | ||
| }) | ||
| .sort(); // Sort for consistent output | ||
|
|
||
| if (files.length === 0) { | ||
| console.warn('Warning: No workflow files found to register'); | ||
| } | ||
|
|
||
| // Generate imports | ||
| const imports = files | ||
| .map((file) => { | ||
| const identifier = generateSafeIdentifier(file); | ||
| // Use .js extension for ESM imports | ||
| const importPath = `./workflows/${file.replace(/\.ts$/, '.js')}`; | ||
| return `import * as ${identifier} from '${importPath}';`; | ||
| }) | ||
| .join('\n'); | ||
|
|
||
| // Generate registry object entries | ||
| const registryEntries = files | ||
| .map((file) => { | ||
| const identifier = generateSafeIdentifier(file); | ||
| return ` 'workflows/${file}': ${identifier},`; | ||
| }) | ||
| .join('\n'); | ||
|
|
||
| // Generate full content | ||
| const content = `// Auto-generated by workbench/scripts/generate-workflows-registry.js | ||
| // Do not edit this file manually - it will be regenerated on build | ||
| ${imports} | ||
| export const allWorkflows = { | ||
| ${registryEntries} | ||
| } as const; |
There was a problem hiding this comment.
The workflow registry generation script removes backward compatibility mappings that existed in the manually-maintained registries, causing UI code to fail when it tries to trigger workflows using old key names. Multiple workbenches have UI code that references workflows/0_calc.ts, but the auto-generated registries only contain workflows/0_demo.ts, resulting in 404 errors at runtime.
View Details
📝 Patch Details
diff --git a/workbench/hono/_workflows.ts b/workbench/hono/_workflows.ts
index 949d80f..f7b322e 100644
--- a/workbench/hono/_workflows.ts
+++ b/workbench/hono/_workflows.ts
@@ -23,4 +23,5 @@ export const allWorkflows = {
'workflows/7_full.ts': workflow_7_full,
'workflows/98_duplicate_case.ts': workflow_98_duplicate_case,
'workflows/99_e2e.ts': workflow_99_e2e,
+ 'workflows/0_calc.ts': workflow_0_demo,
} as const;
diff --git a/workbench/nitro-v3/_workflows.ts b/workbench/nitro-v3/_workflows.ts
index 949d80f..f7b322e 100644
--- a/workbench/nitro-v3/_workflows.ts
+++ b/workbench/nitro-v3/_workflows.ts
@@ -23,4 +23,5 @@ export const allWorkflows = {
'workflows/7_full.ts': workflow_7_full,
'workflows/98_duplicate_case.ts': workflow_98_duplicate_case,
'workflows/99_e2e.ts': workflow_99_e2e,
+ 'workflows/0_calc.ts': workflow_0_demo,
} as const;
diff --git a/workbench/nuxt/_workflows.ts b/workbench/nuxt/_workflows.ts
index 949d80f..f7b322e 100644
--- a/workbench/nuxt/_workflows.ts
+++ b/workbench/nuxt/_workflows.ts
@@ -23,4 +23,5 @@ export const allWorkflows = {
'workflows/7_full.ts': workflow_7_full,
'workflows/98_duplicate_case.ts': workflow_98_duplicate_case,
'workflows/99_e2e.ts': workflow_99_e2e,
+ 'workflows/0_calc.ts': workflow_0_demo,
} as const;
diff --git a/workbench/scripts/generate-workflows-registry.js b/workbench/scripts/generate-workflows-registry.js
index fe6e3dd..5018bff 100644
--- a/workbench/scripts/generate-workflows-registry.js
+++ b/workbench/scripts/generate-workflows-registry.js
@@ -71,6 +71,17 @@ function generateRegistry() {
})
.join('\n');
+ // Add backward compatibility aliases
+ const aliases = [];
+
+ // If 0_demo.ts exists, add an alias for 0_calc.ts for backward compatibility
+ // (0_demo.ts contains calc function)
+ if (files.includes('0_demo.ts')) {
+ aliases.push(` 'workflows/0_calc.ts': workflow_0_demo,`);
+ }
+
+ const allEntries = [registryEntries, ...aliases].join('\n');
+
// Generate full content
const content = `// Auto-generated by workbench/scripts/generate-workflows-registry.js
// Do not edit this file manually - it will be regenerated on build
@@ -78,7 +89,7 @@ function generateRegistry() {
${imports}
export const allWorkflows = {
-${registryEntries}
+${allEntries}
} as const;
`;
diff --git a/workbench/vite/_workflows.ts b/workbench/vite/_workflows.ts
index 949d80f..f7b322e 100644
--- a/workbench/vite/_workflows.ts
+++ b/workbench/vite/_workflows.ts
@@ -23,4 +23,5 @@ export const allWorkflows = {
'workflows/7_full.ts': workflow_7_full,
'workflows/98_duplicate_case.ts': workflow_98_duplicate_case,
'workflows/99_e2e.ts': workflow_99_e2e,
+ 'workflows/0_calc.ts': workflow_0_demo,
} as const;
Analysis
Backward compatibility alias removed from auto-generated workflow registries
What fails: The workflow registry auto-generation script stopped creating backward compatibility aliases. UI code in workbench/vite, workbench/nuxt, workbench/nitro-v3, and workbench/hono reference workflows/0_calc.ts, but the auto-generated registries only contain workflows/0_demo.ts, causing 404 errors when the API tries to look up the workflow file.
How to reproduce:
- Trigger a workflow from workbench/vite/index.html (line 30)
- The UI sends request:
/api/trigger?workflowFile=workflows/0_calc.ts&workflowFn=calc&args=2 - The API looks up
allWorkflows['workflows/0_calc.ts']in the generated registry - Result: 404 error "Workflow file 'workflows/0_calc.ts' not found"
What was expected: The manually-maintained registries (commit fb8153b) included both:
'workflows/0_demo.ts': demo(canonical name)'workflows/0_calc.ts': demo(backward compatibility alias, with comment "// 0_demo.ts contains calc function")
The auto-generation script (commit 7f993bc) removed the alias, breaking existing UI code.
Fix: Modified workbench/scripts/generate-workflows-registry.js to add 'workflows/0_calc.ts': workflow_0_demo as a backward compatibility alias when 0_demo.ts exists. This restores the mapping that was present in the original manually-maintained registries.
pranaygp
force-pushed
the
pranaygp/normalize-workbenches
branch
from
a11298d to
e75af80
Compare
There was a problem hiding this comment.
🔧 Build Fix:
The import path ../../../_workflows is incorrect and points one directory level too shallow, causing the build to fail when trying to resolve the _workflows module.
View Details
📝 Patch Details
diff --git a/workbench/sveltekit/src/routes/api/trigger/+server.ts b/workbench/sveltekit/src/routes/api/trigger/+server.ts
index e0940ab..d0860ba 100644
--- a/workbench/sveltekit/src/routes/api/trigger/+server.ts
+++ b/workbench/sveltekit/src/routes/api/trigger/+server.ts
@@ -1,7 +1,7 @@
import { json, type RequestHandler } from '@sveltejs/kit';
import { getRun, start } from 'workflow/api';
import { hydrateWorkflowArguments } from 'workflow/internal/serialization';
-import { allWorkflows } from '../../../_workflows';
+import { allWorkflows } from '../../../../_workflows';
export const POST: RequestHandler = async ({ request }) => {
const url = new URL(request.url);
Analysis
Incorrect relative import path causes module resolution failure
What fails: TypeScript/Rollup build fails on src/routes/api/trigger/+server.ts due to incorrect relative import path for _workflows module
How to reproduce:
cd workbench/sveltekit && pnpm run buildResult:
error during build:
Could not resolve "../../../_workflows" from "src/routes/api/trigger/+server.ts"
Expected behavior: The import should resolve to the _workflows.ts file in the project root directory.
pranaygp
force-pushed
the
pranaygp/normalize-workbenches
branch
from
e75af80 to
c44f1f3
Compare
pranaygp
force-pushed
the
pranaygp/normalize-workbenches
branch
from
295c498 to
1976cd2
Compare
| if (!workflowFile) { | ||
| return new Response('No workflowFile query parameter provided', { | ||
| status: 400, | ||
| }); |
There was a problem hiding this comment.
The validation checks for workflowFile and workflowFn are unreachable dead code because these variables are always assigned default values via the || operator before the checks execute.
View Details
📝 Patch Details
diff --git a/workbench/sveltekit/src/routes/api/trigger/+server.ts b/workbench/sveltekit/src/routes/api/trigger/+server.ts
index ab50a6b..7f17345 100644
--- a/workbench/sveltekit/src/routes/api/trigger/+server.ts
+++ b/workbench/sveltekit/src/routes/api/trigger/+server.ts
@@ -11,11 +11,6 @@ export const POST: RequestHandler = async ({ request }) => {
const url = new URL(request.url);
const workflowFile =
url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
- if (!workflowFile) {
- return new Response('No workflowFile query parameter provided', {
- status: 400,
- });
- }
const workflows = allWorkflows[workflowFile as keyof typeof allWorkflows];
if (!workflows) {
return new Response(`Workflow file "${workflowFile}" not found`, {
@@ -24,11 +19,6 @@ export const POST: RequestHandler = async ({ request }) => {
}
const workflowFn = url.searchParams.get('workflowFn') || 'simple';
- if (!workflowFn) {
- return new Response('No workflow query parameter provided', {
- status: 400,
- });
- }
const workflow = workflows[workflowFn as keyof typeof workflows];
if (!workflow) {
return new Response(`Workflow "${workflowFn}" not found`, { status: 400 });
Analysis
Dead code validation checks removed from trigger endpoint
What fails: Lines 14-17 and 27-30 in workbench/sveltekit/src/routes/api/trigger/+server.ts contain unreachable validation code. The if (!workflowFile) and if (!workflowFn) checks never execute because the variables are assigned default values via the || operator on the preceding lines, guaranteeing they are always truthy strings.
How to reproduce:
// In +server.ts:
const workflowFile = url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
if (!workflowFile) { // This condition is never true
return new Response('No workflowFile query parameter provided', { status: 400 });
}Testing with Node.js:
const url = new URL('http://example.com/trigger');
const workflowFile = url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
console.log(!workflowFile); // Always falseResult: The validation blocks are dead code that never execute. No matter the input (missing parameter, empty string, or value), the resulting variable is always a non-empty string.
Expected: Dead code should be removed. The || operator ensures variables always have values, making subsequent validation checks redundant and misleading.
Fix: Removed lines 14-17 (workflowFile validation) and lines 27-30 (workflowFn validation) since the default values via || operator provide sufficient guarantees.
| if (!workflowFile) { | ||
| return new Response('No workflowFile query parameter provided', { | ||
| status: 400, | ||
| }); |
There was a problem hiding this comment.
Same issue as SvelteKit - the validation checks for workflowFile and workflowFn are unreachable dead code because these variables are always assigned default values via the || operator before the checks execute.
View Details
📝 Patch Details
diff --git a/workbench/nitro-v2/server/api/trigger.post.ts b/workbench/nitro-v2/server/api/trigger.post.ts
index b5d85fe..d4d01ce 100644
--- a/workbench/nitro-v2/server/api/trigger.post.ts
+++ b/workbench/nitro-v2/server/api/trigger.post.ts
@@ -8,11 +8,6 @@ export default defineEventHandler(async (event) => {
const workflowFile =
url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
- if (!workflowFile) {
- return new Response('No workflowFile query parameter provided', {
- status: 400,
- });
- }
const workflows = allWorkflows[workflowFile as keyof typeof allWorkflows];
if (!workflows) {
return new Response(`Workflow file "${workflowFile}" not found`, {
@@ -21,11 +16,6 @@ export default defineEventHandler(async (event) => {
}
const workflowFn = url.searchParams.get('workflowFn') || 'simple';
- if (!workflowFn) {
- return new Response('No workflow query parameter provided', {
- status: 400,
- });
- }
const workflow = workflows[workflowFn as keyof typeof workflows];
if (!workflow) {
return new Response(`Workflow "${workflowFn}" not found`, { status: 400 });
diff --git a/workbench/nitro-v3/routes/api/trigger.post.ts b/workbench/nitro-v3/routes/api/trigger.post.ts
index 2cf0025..f2654c9 100644
--- a/workbench/nitro-v3/routes/api/trigger.post.ts
+++ b/workbench/nitro-v3/routes/api/trigger.post.ts
@@ -5,11 +5,6 @@ import { allWorkflows } from '../../_workflows.js';
export default async ({ req, url }: { req: Request; url: URL }) => {
const workflowFile =
url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
- if (!workflowFile) {
- return new Response('No workflowFile query parameter provided', {
- status: 400,
- });
- }
const workflows = allWorkflows[workflowFile as keyof typeof allWorkflows];
if (!workflows) {
return new Response(`Workflow file "${workflowFile}" not found`, {
@@ -18,11 +13,6 @@ export default async ({ req, url }: { req: Request; url: URL }) => {
}
const workflowFn = url.searchParams.get('workflowFn') || 'simple';
- if (!workflowFn) {
- return new Response('No workflow query parameter provided', {
- status: 400,
- });
- }
const workflow = workflows[workflowFn as keyof typeof workflows];
if (!workflow) {
return new Response(`Workflow "${workflowFn}" not found`, { status: 400 });
diff --git a/workbench/nuxt/server/api/trigger.post.ts b/workbench/nuxt/server/api/trigger.post.ts
index b5d85fe..d4d01ce 100644
--- a/workbench/nuxt/server/api/trigger.post.ts
+++ b/workbench/nuxt/server/api/trigger.post.ts
@@ -8,11 +8,6 @@ export default defineEventHandler(async (event) => {
const workflowFile =
url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
- if (!workflowFile) {
- return new Response('No workflowFile query parameter provided', {
- status: 400,
- });
- }
const workflows = allWorkflows[workflowFile as keyof typeof allWorkflows];
if (!workflows) {
return new Response(`Workflow file "${workflowFile}" not found`, {
@@ -21,11 +16,6 @@ export default defineEventHandler(async (event) => {
}
const workflowFn = url.searchParams.get('workflowFn') || 'simple';
- if (!workflowFn) {
- return new Response('No workflow query parameter provided', {
- status: 400,
- });
- }
const workflow = workflows[workflowFn as keyof typeof workflows];
if (!workflow) {
return new Response(`Workflow "${workflowFn}" not found`, { status: 400 });
diff --git a/workbench/vite/routes/api/trigger.post.ts b/workbench/vite/routes/api/trigger.post.ts
index 2cf0025..f2654c9 100644
--- a/workbench/vite/routes/api/trigger.post.ts
+++ b/workbench/vite/routes/api/trigger.post.ts
@@ -5,11 +5,6 @@ import { allWorkflows } from '../../_workflows.js';
export default async ({ req, url }: { req: Request; url: URL }) => {
const workflowFile =
url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
- if (!workflowFile) {
- return new Response('No workflowFile query parameter provided', {
- status: 400,
- });
- }
const workflows = allWorkflows[workflowFile as keyof typeof allWorkflows];
if (!workflows) {
return new Response(`Workflow file "${workflowFile}" not found`, {
@@ -18,11 +13,6 @@ export default async ({ req, url }: { req: Request; url: URL }) => {
}
const workflowFn = url.searchParams.get('workflowFn') || 'simple';
- if (!workflowFn) {
- return new Response('No workflow query parameter provided', {
- status: 400,
- });
- }
const workflow = workflows[workflowFn as keyof typeof workflows];
if (!workflow) {
return new Response(`Workflow "${workflowFn}" not found`, { status: 400 });
Analysis
Unreachable validation checks for workflowFile and workflowFn in trigger endpoints
What fails: The validation checks in trigger.post.ts files are unreachable dead code. The if (!workflowFile) and if (!workflowFn) blocks will never execute because these variables are always assigned non-empty default values via the || operator.
How to reproduce:
// In any trigger.post.ts file:
const workflowFile = url.searchParams.get('workflowFile') || 'workflows/99_e2e.ts';
if (!workflowFile) { // This block never executes
return new Response('No workflowFile query parameter provided', { status: 400 });
}Testing the logic:
- When parameter is provided:
workflowFileis the parameter value (truthy) - When parameter is missing:
workflowFileis'workflows/99_e2e.ts'(truthy) - When parameter is empty string:
workflowFileis'workflows/99_e2e.ts'(truthy)
In all cases, !workflowFile evaluates to false, making the check unreachable.
Result: The validation blocks are dead code that provides no value and creates confusion about the actual behavior (defaults are used, not validation).
Expected: Dead code validation checks should be removed. The actual validation that matters occurs later when checking if the workflow exists in the workflows object.
Files affected:
- workbench/vite/routes/api/trigger.post.ts
- workbench/nitro-v3/routes/api/trigger.post.ts
- workbench/nuxt/server/api/trigger.post.ts
- workbench/nitro-v2/server/api/trigger.post.ts
Fix: Removed the unreachable if (!workflowFile) and if (!workflowFn) validation blocks from all trigger.post.ts files. The subsequent checks for workflow existence in the workflows object provide the necessary validation.
pranaygp
force-pushed
the
pranaygp/normalize-workbenches
branch
from
1976cd2 to
26edd59
Compare
The various workbenches have gone out of sync. Just trying to normalize and clean them up for better testing and internal DX
This is part 3 of 3 in a stack made with GitButler: