From 779112a4dfdaaef713f79fb4e70bee1f07984727 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Fri, 30 Jan 2026 15:45:35 -0800 Subject: [PATCH 1/2] Fix discovery of serde classes to detect `[WORKFLOW_SERIALIZE]` and `[WORKFLOW_DESERIALIZE]` computed property usage in bundled code --- .changeset/three-apples-draw.md | 5 + packages/builders/src/base-builder.ts | 11 +- packages/builders/src/transform-utils.test.ts | 137 ++++++++++++++++++ packages/builders/src/transform-utils.ts | 15 +- 4 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 .changeset/three-apples-draw.md diff --git a/.changeset/three-apples-draw.md b/.changeset/three-apples-draw.md new file mode 100644 index 0000000000..0b3e05ffd7 --- /dev/null +++ b/.changeset/three-apples-draw.md @@ -0,0 +1,5 @@ +--- +"@workflow/builders": patch +--- + +Fix discovery of serde classes to detect `[WORKFLOW_SERIALIZE]` and `[WORKFLOW_DESERIALIZE]` computed property usage in bundled code diff --git a/packages/builders/src/base-builder.ts b/packages/builders/src/base-builder.ts index 6a1c43530c..25f3d0d68e 100644 --- a/packages/builders/src/base-builder.ts +++ b/packages/builders/src/base-builder.ts @@ -417,6 +417,7 @@ export abstract class BaseBuilder { entriesToBundle: externalizeNonSteps ? [ ...stepFiles, + ...serdeFiles, ...(resolvedBuiltInSteps ? [resolvedBuiltInSteps] : []), ] : undefined, @@ -520,8 +521,8 @@ export abstract class BaseBuilder { await this.writeDebugFile(outfile, { workflowFiles, serdeOnlyFiles }); // Helper to create import statement from file path - // For workspace/node_modules packages, uses the package name so esbuild - // will resolve through package.json exports with conditions: ['workflow'] + // For packages, uses the package name so esbuild will resolve through + // package.json exports with conditions: ['workflow'] const createImport = (file: string) => { const { importPath, isPackage } = getImportPath( file, @@ -556,7 +557,11 @@ export abstract class BaseBuilder { // calls directly, so we just need to import the files (Map is initialized via banner) const workflowImports = workflowFiles.map(createImport).join('\n'); - // Include serde-only files for class registration side effects + // Include serde-only files for class registration side effects. + // Note: If a package has serde classes that aren't exported via the 'workflow' + // condition, those classes won't be available in the workflow bundle. Packages + // should export all serializable classes (including internal ones used during + // serialization) via their 'workflow' export condition. const serdeImports = serdeOnlyFiles.map(createImport).join('\n'); const imports = serdeImports diff --git a/packages/builders/src/transform-utils.test.ts b/packages/builders/src/transform-utils.test.ts index 127d21ad9a..310e4f7e1a 100644 --- a/packages/builders/src/transform-utils.test.ts +++ b/packages/builders/src/transform-utils.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from 'vitest'; import { + detectWorkflowPatterns, useStepPattern, useWorkflowPattern, + workflowSerdeComputedPropertyPattern, workflowSerdeImportPattern, workflowSerdeSymbolPattern, } from './transform-utils.js'; @@ -195,4 +197,139 @@ describe('transform-utils patterns', () => { expect(workflowSerdeSymbolPattern.test(source)).toBe(true); }); }); + + describe('workflowSerdeComputedPropertyPattern', () => { + it('should match [WORKFLOW_SERIALIZE] computed property', () => { + const source = `static [WORKFLOW_SERIALIZE](instance) {}`; + expect(workflowSerdeComputedPropertyPattern.test(source)).toBe(true); + }); + + it('should match [WORKFLOW_DESERIALIZE] computed property', () => { + const source = `static [WORKFLOW_DESERIALIZE](data) {}`; + expect(workflowSerdeComputedPropertyPattern.test(source)).toBe(true); + }); + + it('should match with whitespace inside brackets', () => { + expect( + workflowSerdeComputedPropertyPattern.test(`[ WORKFLOW_SERIALIZE ]`) + ).toBe(true); + expect( + workflowSerdeComputedPropertyPattern.test(`[ WORKFLOW_DESERIALIZE ]`) + ).toBe(true); + }); + + it('should match in bundled code where symbols are imported from chunks', () => { + // This is the pattern seen in bundled packages like just-bash + const source = ` + import { + WORKFLOW_DESERIALIZE, + WORKFLOW_SERIALIZE + } from "./chunks/chunk-453323QY.js"; + + var Bash = class _Bash { + static [WORKFLOW_SERIALIZE](instance) { + return { fs: instance.fs }; + } + static [WORKFLOW_DESERIALIZE](serialized) { + return Object.create(_Bash.prototype, { + fs: { value: serialized.fs } + }); + } + }; + `; + expect(workflowSerdeComputedPropertyPattern.test(source)).toBe(true); + // Note: import pattern won't match because it's from a chunk, not @workflow/serde + expect(workflowSerdeImportPattern.test(source)).toBe(false); + }); + + it('should not match partial names', () => { + expect( + workflowSerdeComputedPropertyPattern.test(`[WORKFLOW_SERIALIZE_EXTRA]`) + ).toBe(false); + expect( + workflowSerdeComputedPropertyPattern.test(`[MY_WORKFLOW_SERIALIZE]`) + ).toBe(false); + }); + + it('should not match string literals', () => { + expect( + workflowSerdeComputedPropertyPattern.test(`['WORKFLOW_SERIALIZE']`) + ).toBe(false); + expect( + workflowSerdeComputedPropertyPattern.test(`["WORKFLOW_DESERIALIZE"]`) + ).toBe(false); + }); + }); + + describe('detectWorkflowPatterns', () => { + it('should detect hasSerde for @workflow/serde import', () => { + const source = `import { WORKFLOW_SERIALIZE } from '@workflow/serde';`; + const result = detectWorkflowPatterns(source); + expect(result.hasSerde).toBe(true); + expect(result.hasSerdeImport).toBe(true); + }); + + it('should detect hasSerde for Symbol.for pattern', () => { + const source = `static [Symbol.for('workflow-serialize')](instance) {}`; + const result = detectWorkflowPatterns(source); + expect(result.hasSerde).toBe(true); + expect(result.hasSerdeSymbol).toBe(true); + }); + + it('should detect hasSerde for computed property pattern', () => { + const source = `static [WORKFLOW_SERIALIZE](instance) {}`; + const result = detectWorkflowPatterns(source); + expect(result.hasSerde).toBe(true); + }); + + it('should detect hasSerde for bundled third-party packages', () => { + // Simulates bundled output from packages like just-bash + const source = ` + import { + WORKFLOW_DESERIALIZE, + WORKFLOW_SERIALIZE + } from "./chunks/chunk-ABC123.js"; + + var MyClass = class { + static [WORKFLOW_SERIALIZE](instance) { + return { data: instance.data }; + } + static [WORKFLOW_DESERIALIZE](serialized) { + return new MyClass(serialized.data); + } + }; + `; + const result = detectWorkflowPatterns(source); + expect(result.hasSerde).toBe(true); + }); + + it('should not detect hasSerde for unrelated code', () => { + const source = ` + export class RegularClass { + constructor(value) { + this.value = value; + } + } + `; + const result = detectWorkflowPatterns(source); + expect(result.hasSerde).toBe(false); + }); + + it('should detect both directive and serde patterns', () => { + const source = ` + 'use step'; + import { WORKFLOW_SERIALIZE } from '@workflow/serde'; + + export class Point { + static [WORKFLOW_SERIALIZE](instance) { + return { x: instance.x }; + } + } + `; + const result = detectWorkflowPatterns(source); + expect(result.hasDirective).toBe(true); + expect(result.hasUseStep).toBe(true); + expect(result.hasSerde).toBe(true); + }); + }); }); diff --git a/packages/builders/src/transform-utils.ts b/packages/builders/src/transform-utils.ts index 22d7df3795..2b7d459dff 100644 --- a/packages/builders/src/transform-utils.ts +++ b/packages/builders/src/transform-utils.ts @@ -16,6 +16,12 @@ export const workflowSerdeImportPattern = /from\s+(['"])@workflow\/serde\1/; export const workflowSerdeSymbolPattern = /Symbol\.for\s*\(\s*(['"])workflow-(?:serialize|deserialize)\1\s*\)/; +// Matches usage of WORKFLOW_SERIALIZE or WORKFLOW_DESERIALIZE as computed property names +// e.g.: static [WORKFLOW_SERIALIZE](instance) { ... } +// This catches cases where the symbols are imported and used (even if bundled/re-exported) +export const workflowSerdeComputedPropertyPattern = + /\[\s*WORKFLOW_(?:SERIALIZE|DESERIALIZE)\s*\]/; + // Pattern to detect generated workflow route files that should be excluded // These files are generated by the build process and should not be re-processed export const generatedWorkflowPathPattern = @@ -56,6 +62,8 @@ export function detectWorkflowPatterns(source: string): WorkflowPatternMatch { const hasUseStep = useStepPattern.test(source); const hasSerdeImport = workflowSerdeImportPattern.test(source); const hasSerdeSymbol = workflowSerdeSymbolPattern.test(source); + const hasSerdeComputedProperty = + workflowSerdeComputedPropertyPattern.test(source); return { hasUseWorkflow, @@ -63,7 +71,7 @@ export function detectWorkflowPatterns(source: string): WorkflowPatternMatch { hasSerdeImport, hasSerdeSymbol, hasDirective: hasUseWorkflow || hasUseStep, - hasSerde: hasSerdeImport || hasSerdeSymbol, + hasSerde: hasSerdeImport || hasSerdeSymbol || hasSerdeComputedProperty, }; } @@ -123,7 +131,8 @@ export function shouldTransformFile( /** * Combined regex pattern for turbopack content matching. * Uses backreferences to ensure matching quote types. - * Matches: 'use workflow', 'use step', @workflow/serde imports, and Symbol.for serialization symbols + * Matches: 'use workflow', 'use step', @workflow/serde imports, Symbol.for serialization symbols, + * and WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE computed property usage */ export const turbopackContentPattern = - /(use workflow|use step|from\s+(['"])@workflow\/serde\2|Symbol\.for\s*\(\s*(['"])workflow-(?:serialize|deserialize)\3\s*\))/; + /(use workflow|use step|from\s+(['"])@workflow\/serde\2|Symbol\.for\s*\(\s*(['"])workflow-(?:serialize|deserialize)\3\s*\)|\[\s*WORKFLOW_(?:SERIALIZE|DESERIALIZE)\s*\])/; From 52c4669ba8bc4877f9ac94c0286f7b15ee2be640 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Wed, 4 Feb 2026 23:51:27 -0800 Subject: [PATCH 2/2] docs: clarify how to make classes workflow compatible Update comment to describe the preferred approach: adding 'use step' directives to instance methods that rely on Node.js imports, allowing those methods to be replaced with step proxy functions and the Node.js imports to be dead code eliminated. --- packages/builders/src/base-builder.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/builders/src/base-builder.ts b/packages/builders/src/base-builder.ts index 25f3d0d68e..429da67684 100644 --- a/packages/builders/src/base-builder.ts +++ b/packages/builders/src/base-builder.ts @@ -557,11 +557,7 @@ export abstract class BaseBuilder { // calls directly, so we just need to import the files (Map is initialized via banner) const workflowImports = workflowFiles.map(createImport).join('\n'); - // Include serde-only files for class registration side effects. - // Note: If a package has serde classes that aren't exported via the 'workflow' - // condition, those classes won't be available in the workflow bundle. Packages - // should export all serializable classes (including internal ones used during - // serialization) via their 'workflow' export condition. + // Include serde-only files for class registration side effects const serdeImports = serdeOnlyFiles.map(createImport).join('\n'); const imports = serdeImports