Skip to content

Commit 906f7c1

Browse files
TooTallNateijjk
andauthored
refactor(next): remove step file copy mechanism from deferred builder (vercel#1796)
* refactor(next): remove step file copy mechanism from deferred builder Step sources are now imported directly into the generated `step/route.js` using the same `getImportPath`-based logic already used for serde files. This is possible because the SWC plugin's client mode was merged into step mode (vercel#1686), so the workflow loader always runs in step mode and transforms every file it sees, including those from packages. Removed: - `__workflow_step_files__/` per-file copies with hashed names, metadata comments, inline source maps, and bare-specifier rewriting via `enhanced-resolve` - `createResponseBuiltinsStepFile`; builtins are now imported via `require.resolve('workflow/internal/builtins')` at build time - `step-copy-utils.ts` and all copy-specific branches in `loader.ts` - `enhanced-resolve` dependency The deferred builder still removes the legacy `__workflow_step_files__/` directory on boot so upgrades leave no stale artifacts behind. Dev tests that inspected copied step file contents now inspect the manifest.json entries (which list every discovered step keyed by source path). * refactor(next): scrub historical phrasing from code comments * test: update next step stack expectations --------- Co-authored-by: JJ Kasper <jj@jjsweb.site>
1 parent 0ec805a commit 906f7c1

9 files changed

Lines changed: 180 additions & 691 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@workflow/next": patch
3+
---
4+
5+
Simplify the deferred builder by importing step sources directly into the generated `step/route.js`, matching how serde files are handled.

packages/core/e2e/dev.test.ts

Lines changed: 41 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,22 @@ export function createDevTests(config?: DevTestConfig) {
4444
);
4545
const testWorkflowFile = finalConfig.testWorkflowFile ?? '3_streams.ts';
4646
const workflowsDir = finalConfig.workflowsDir ?? 'workflows';
47-
const supportsDeferredStepCopies = generatedStep.includes(
47+
const usesDeferredBuilder = generatedStep.includes(
4848
path.join('.well-known', 'workflow', 'v1', 'step', 'route.js')
4949
);
50+
const workflowManifestPath = path.join(
51+
appPath,
52+
'app/.well-known/workflow/v1/manifest.json'
53+
);
54+
const readManifestStepFunctionNames = async (): Promise<string[]> => {
55+
const manifestJson = await fs.readFile(workflowManifestPath, 'utf8');
56+
const manifest = JSON.parse(manifestJson) as {
57+
steps?: Record<string, Record<string, unknown>>;
58+
};
59+
return Object.values(manifest.steps || {}).flatMap((entry) =>
60+
Object.keys(entry)
61+
);
62+
};
5063
const restoreFiles: Array<{ path: string; content: string }> = [];
5164

5265
const fetchWithTimeout = (pathname: string) => {
@@ -192,10 +205,6 @@ export async function myNewStep() {
192205
`
193206
);
194207
restoreFiles.push({ path: stepFile, content });
195-
const copiedStepDir = path.join(
196-
path.dirname(generatedStep),
197-
'__workflow_step_files__'
198-
);
199208

200209
await pollUntil({
201210
description: 'generated step outputs to include myNewStep',
@@ -205,28 +214,20 @@ export async function myNewStep() {
205214
return;
206215
}
207216

208-
const copiedStepFileNames = await fs.readdir(copiedStepDir);
209-
const copiedStepContents = await Promise.all(
210-
copiedStepFileNames.map(async (copiedStepFileName) => {
211-
const copiedStepFilePath = path.join(
212-
copiedStepDir,
213-
copiedStepFileName
214-
);
215-
const copiedStepStats = await fs.stat(copiedStepFilePath);
216-
if (!copiedStepStats.isFile()) {
217-
return '';
218-
}
219-
return await fs.readFile(copiedStepFilePath, 'utf8');
220-
})
221-
);
222-
expect(
223-
copiedStepContents.some((content) => content.includes('myNewStep'))
224-
).toBe(true);
217+
// The deferred builder regenerates manifest.json on every rebuild.
218+
// Check the manifest for the new step function name.
219+
if (usesDeferredBuilder) {
220+
const manifestFunctionNames = await readManifestStepFunctionNames();
221+
expect(manifestFunctionNames).toContain('myNewStep');
222+
return;
223+
}
224+
225+
throw new Error('myNewStep not found in generated step outputs');
225226
},
226227
});
227228
});
228229

229-
test.skipIf(!supportsDeferredStepCopies)(
230+
test.skipIf(!usesDeferredBuilder)(
230231
'should rebuild on imported step dependency change',
231232
{ timeout: 60_000 },
232233
async () => {
@@ -250,36 +251,14 @@ export async function ${marker}() {
250251
);
251252
restoreFiles.push({ path: importedStepFile, content });
252253

253-
const copiedStepDir = path.join(
254-
path.dirname(generatedStep),
255-
'__workflow_step_files__'
256-
);
257-
258254
await pollUntil({
259255
description:
260-
'copied deferred step files to include imported step hot-reload marker',
256+
'manifest.json to include imported step hot-reload marker',
261257
timeoutMs: 50_000,
262258
check: async () => {
263259
await triggerWorkflowRun('importedStepOnlyWorkflow');
264-
const copiedStepFileNames = await fs.readdir(copiedStepDir);
265-
const copiedStepContents = await Promise.all(
266-
copiedStepFileNames.map(async (copiedStepFileName) => {
267-
const copiedStepFilePath = path.join(
268-
copiedStepDir,
269-
copiedStepFileName
270-
);
271-
const copiedStepStats = await fs.stat(copiedStepFilePath);
272-
if (!copiedStepStats.isFile()) {
273-
return '';
274-
}
275-
return await fs.readFile(copiedStepFilePath, 'utf8');
276-
})
277-
);
278-
expect(
279-
copiedStepContents.some((copiedStepContent) =>
280-
copiedStepContent.includes(marker)
281-
)
282-
).toBe(true);
260+
const manifestFunctionNames = await readManifestStepFunctionNames();
261+
expect(manifestFunctionNames).toContain(marker);
283262
},
284263
});
285264
}
@@ -330,7 +309,7 @@ ${apiFileContent}`
330309
}
331310
);
332311

333-
test.skipIf(!supportsDeferredStepCopies)(
312+
test.skipIf(!usesDeferredBuilder)(
334313
'should include steps discovered from workflow imports',
335314
{ timeout: 30_000 },
336315
async () => {
@@ -378,57 +357,28 @@ export async function discoveredViaWorkflowStep() {
378357
${apiFileContent}`
379358
);
380359

381-
const copiedStepDir = path.join(
382-
path.dirname(generatedStep),
383-
'__workflow_step_files__'
384-
);
385-
386360
await pollUntil({
387361
description:
388-
'copied deferred step files to include discoveredViaWorkflowStep',
362+
'manifest.json to include discoveredViaWorkflowStep after discovery',
389363
timeoutMs: 25_000,
390364
check: async () => {
391365
await fetchWithTimeout('/api/chat');
392-
const copiedStepFileNames = await fs.readdir(copiedStepDir);
393-
const copiedStepContents = await Promise.all(
394-
copiedStepFileNames.map(async (copiedStepFileName) => {
395-
const copiedStepFilePath = path.join(
396-
copiedStepDir,
397-
copiedStepFileName
398-
);
399-
const copiedStepStats = await fs.stat(copiedStepFilePath);
400-
if (!copiedStepStats.isFile()) {
401-
return '';
402-
}
403-
return await fs.readFile(copiedStepFilePath, 'utf8');
404-
})
366+
const manifestFunctionNames = await readManifestStepFunctionNames();
367+
expect(manifestFunctionNames).toContain(
368+
'discoveredViaWorkflowStep'
405369
);
406-
expect(
407-
copiedStepContents.some((content) =>
408-
content.includes('discoveredViaWorkflowStep')
409-
)
410-
).toBe(true);
411370
},
412371
});
413372
}
414373
);
415374

416-
test.skipIf(!supportsDeferredStepCopies)(
417-
'should copy package step sources discovered via manifest entries',
375+
test.skipIf(!usesDeferredBuilder)(
376+
'should reference package step sources discovered via manifest entries',
418377
{ timeout: 30_000 },
419378
async () => {
420-
const workflowManifestPath = path.join(
421-
appPath,
422-
'app/.well-known/workflow/v1/manifest.json'
423-
);
424-
const copiedStepDir = path.join(
425-
path.dirname(generatedStep),
426-
'__workflow_step_files__'
427-
);
428-
429379
await pollUntil({
430380
description:
431-
'copied deferred step files to include @workflow/ai package steps',
381+
'generated step route to reference @workflow/ai package steps',
432382
timeoutMs: 25_000,
433383
check: async () => {
434384
await fetchWithTimeout('/api/chat');
@@ -446,24 +396,13 @@ ${apiFileContent}`
446396
)
447397
).toBe(true);
448398

449-
const copiedStepFileNames = await fs.readdir(copiedStepDir);
450-
const copiedStepContents = await Promise.all(
451-
copiedStepFileNames.map(async (copiedStepFileName) => {
452-
const copiedStepFilePath = path.join(
453-
copiedStepDir,
454-
copiedStepFileName
455-
);
456-
const copiedStepStats = await fs.stat(copiedStepFilePath);
457-
if (!copiedStepStats.isFile()) {
458-
return '';
459-
}
460-
return await fs.readFile(copiedStepFilePath, 'utf8');
461-
})
462-
);
399+
// Package step sources are imported directly (not copied). Verify
400+
// the generated step route imports the @workflow/ai package or
401+
// otherwise references `durable-agent` via its resolved path.
402+
const stepRouteContent = await fs.readFile(generatedStep, 'utf8');
463403
expect(
464-
copiedStepContents.some((content) =>
465-
content.includes('async function closeStream')
466-
)
404+
stepRouteContent.includes('@workflow/ai') ||
405+
stepRouteContent.includes('durable-agent')
467406
).toBe(true);
468407
},
469408
});

packages/core/e2e/e2e.test.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
getCollectedRunIds,
3434
getProtectionBypassHeaders,
3535
getWorkflowMetadata,
36+
hasNestedStepStackFrames,
3637
hasStepSourceMaps,
3738
hasWorkflowSourceMaps,
3839
isLocalDeployment,
@@ -1014,8 +1015,11 @@ describe('e2e', () => {
10141015
expect(result.message).toContain(
10151016
'Step error from imported helper module'
10161017
);
1017-
// Stack trace propagates to caught error with function names and source file
1018-
expect(result.stack).toContain('throwErrorFromStep');
1018+
// Stack trace propagates to caught error with function names and source file.
1019+
// Some production bundlers collapse non-exported helper frames.
1020+
if (hasNestedStepStackFrames()) {
1021+
expect(result.stack).toContain('throwErrorFromStep');
1022+
}
10191023
expect(result.stack).toContain('stepThatThrowsFromHelper');
10201024
expect(result.stack).not.toContain('evalmachine');
10211025

@@ -1035,7 +1039,9 @@ describe('e2e', () => {
10351039
s.stepName.includes('stepThatThrowsFromHelper')
10361040
);
10371041
expect(failedStep.status).toBe('failed');
1038-
expect(failedStep.error.stack).toContain('throwErrorFromStep');
1042+
if (hasNestedStepStackFrames()) {
1043+
expect(failedStep.error.stack).toContain('throwErrorFromStep');
1044+
}
10391045
expect(failedStep.error.stack).toContain(
10401046
'stepThatThrowsFromHelper'
10411047
);

packages/core/e2e/utils.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,15 @@ export function isLocalDeployment(): boolean {
4949
* get rid of this strange matrix
5050
*/
5151
export function hasStepSourceMaps(): boolean {
52-
// Next.js does not consume inline sourcemaps AT ALL for step bundles
53-
// TODO: we need to fix this
5452
const appName = process.env.APP_NAME as string;
55-
if (['nextjs-webpack', 'nextjs-turbopack'].includes(appName)) {
53+
// Turbopack still does not consume inline sourcemaps for step bundles.
54+
// TODO: we need to fix this
55+
if (appName === 'nextjs-turbopack') {
56+
return false;
57+
}
58+
// Webpack dev imports original step sources directly, so source filenames are
59+
// available. Production-style builds still do not expose them consistently.
60+
if (appName === 'nextjs-webpack' && !process.env.DEV_TEST_CONFIG) {
5661
return false;
5762
}
5863

@@ -78,6 +83,17 @@ export function hasStepSourceMaps(): boolean {
7883
return true;
7984
}
8085

86+
/**
87+
* Checks if non-exported nested helper function names are expected to survive
88+
* in step error stack traces.
89+
*/
90+
export function hasNestedStepStackFrames(): boolean {
91+
const appName = process.env.APP_NAME as string;
92+
// Turbopack production-style builds can collapse the non-exported helper
93+
// frame while preserving the exported step frame and error message.
94+
return appName !== 'nextjs-turbopack' || Boolean(process.env.DEV_TEST_CONFIG);
95+
}
96+
8197
/**
8298
* Checks if workflow error source maps are expected to work in the current test environment.
8399
* TODO: ideally it should work consistently everywhere and we should fix the issues and

packages/next/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
"@workflow/builders": "workspace:*",
3838
"@workflow/core": "workspace:*",
3939
"@workflow/swc-plugin": "workspace:*",
40-
"enhanced-resolve": "catalog:",
4140
"semver": "catalog:",
4241
"watchpack": "2.5.1"
4342
},

0 commit comments

Comments
 (0)