Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/keep-json-import-attributes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@workflow/builders': patch
---

Keep JSON import attributes in generated bundles, and stop bundling the builder's own serde checker into step and workflow bundles
18 changes: 12 additions & 6 deletions packages/builders/src/apply-swc-transform.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createRequire } from 'node:module';
import { dirname, isAbsolute, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { transform } from '@swc/core';
import { transform, type WasmPlugin } from '@swc/core';
import { getDecoratorOptionsForDirectory } from './config-helpers.js';
import { resolveModuleSpecifier } from './module-specifier.js';

Expand Down Expand Up @@ -98,6 +98,10 @@ export async function applySwcTransform(
resolvedModuleSpecifierRoot
);

const plugins: WasmPlugin[] | undefined = mode
? [[swcPluginPath, { mode, moduleSpecifier }]]
: undefined;

// Transform with SWC to support syntax esbuild doesn't
const result = await transform(source, {
filename,
Expand All @@ -117,11 +121,13 @@ export async function applySwcTransform(
}),
},
target: 'es2022',
experimental: mode
? {
plugins: [[swcPluginPath, { mode, moduleSpecifier }]],
}
: undefined,
experimental: {
// SWC drops import attributes (`with { type: 'json' }`) by default.
// Imports that stay external in the emitted bundle are then rejected
// by Node's ESM loader with ERR_IMPORT_ATTRIBUTE_MISSING.
keepImportAttributes: true,
plugins,
},
transform: {
react: {
runtime: 'preserve',
Expand Down
29 changes: 26 additions & 3 deletions packages/builders/src/base-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import {
} from './constants.js';
import { getEsbuildTsconfigOptions } from './esbuild-tsconfig.js';
import {
fastDiscoverEntries,
type DiscoveredEntries,
fastDiscoverEntries,
} from './fast-discovery.js';
import {
hashManifestSource,
Expand All @@ -51,6 +51,18 @@ import { extractWorkflowGraphs } from './workflows-extractor.js';
const enhancedResolve = promisify(enhancedResolveOriginal);
const require = createRequire(import.meta.url);

/**
* esbuild treats import attributes (`import data from './x.json' with
* { type: 'json' }`) as unsupported for the `es2022` target and drops them
* from the output. A JSON import that stays external is then rejected by
* Node's ESM loader with ERR_IMPORT_ATTRIBUTE_MISSING. Every Node.js version
* the SDK supports accepts the `with` keyword, so each bundle that Node loads
* directly opts in.
*/
const NODE_ESBUILD_SUPPORTED = {
'import-attributes': true,
} as const;

export type { DiscoveredEntries } from './fast-discovery.js';

/**
Expand Down Expand Up @@ -905,6 +917,7 @@ export const __steps_registered = true;
platform: 'node',
conditions: ['node'],
target: 'es2022',
supported: NODE_ESBUILD_SUPPORTED,
write: true,
treeShaking: true,
keepNames: true,
Expand Down Expand Up @@ -1035,6 +1048,8 @@ export const __steps_registered = true;
manifest: WorkflowManifest;
interimBundleCtx?: esbuild.BuildContext;
bundleFinal?: (interimBundleResult: string) => Promise<void>;
/** The raw workflow VM code (before wrapping with entrypoint) */
interimBundleText?: string;
}> {
this.startWorkflowBuildTimer();

Expand Down Expand Up @@ -1123,6 +1138,9 @@ export const __steps_registered = true;
platform: 'neutral', // The platform is neither node nor browser
mainFields: ['module', 'main'], // To support npm style imports
conditions: ['workflow'], // Allow packages to export 'workflow' compliant versions
// No `supported: NODE_ESBUILD_SUPPORTED` here: this bundle runs in the
// workflow VM, which has no module loader, and it has no `external`, so
// every JSON import is inlined and no import attribute reaches the output.
target: 'es2022',
write: false,
treeShaking: true,
Expand Down Expand Up @@ -1316,6 +1334,7 @@ ${createWorkflowRouteHandlersCode(`workflowEntrypoint(workflowCode${workflowEntr
format,
platform: 'node',
target: 'es2022',
supported: NODE_ESBUILD_SUPPORTED,
write: true,
keepNames: true,
minify: false,
Expand All @@ -1335,17 +1354,19 @@ ${createWorkflowRouteHandlersCode(`workflowEntrypoint(workflowCode${workflowEntr
`${Date.now() - bundleStartTime}ms`
);
};
await bundleFinal(interimBundle.outputFiles[0].text);
const interimBundleText = interimBundle.outputFiles[0].text;
await bundleFinal(interimBundleText);

if (keepInterimBundleContext) {
shouldDisposeInterimBundleCtx = false;
return {
manifest: workflowManifest,
interimBundleCtx,
bundleFinal,
interimBundleText,
};
}
return { manifest: workflowManifest };
return { manifest: workflowManifest, interimBundleText };
} catch (error) {
shouldDisposeInterimBundleCtx = true;
throw error;
Expand Down Expand Up @@ -1462,6 +1483,7 @@ ${createWorkflowRouteHandlersCode(`workflowEntrypoint(workflowCode${workflowEntr
platform: 'node',
jsx: 'preserve',
target: 'es2022',
supported: NODE_ESBUILD_SUPPORTED,
write: true,
treeShaking: true,
external: ['@workflow/core'],
Expand Down Expand Up @@ -1568,6 +1590,7 @@ export const OPTIONS = handler;`;
platform: 'node',
conditions: ['import', 'module', 'node', 'default'],
target: 'es2022',
supported: NODE_ESBUILD_SUPPORTED,
write: true,
treeShaking: true,
keepNames: true,
Expand Down
47 changes: 47 additions & 0 deletions packages/builders/src/fast-discovery.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
mkdirSync,
mkdtempSync,
readFileSync,
realpathSync,
rmSync,
writeFileSync,
Expand Down Expand Up @@ -608,6 +609,52 @@ export const WORKFLOW_SERIALIZE = Symbol.for('workflow-serialize');
expect(discovered.discoveredSerdeFiles).toEqual(new Set());
});

it('ignores serde examples that only appear inside string literals', async () => {
const entryFile = join(testRoot, 'src', 'entry.ts');
const messagesFile = join(testRoot, 'src', 'messages.ts');

writeFile(entryFile, `import './messages';\n`);
writeFile(
messagesFile,
`import { WORKFLOW_SERIALIZE } from '@workflow/serde';

export const hint =
'Define it as: static [WORKFLOW_SERIALIZE](value) { ... }';
export const symbolHint = "static [Symbol.for('workflow-serialize')](value)";
export const templateHint = \`static [WS](value) for \${WORKFLOW_SERIALIZE.toString()}\`;
`
);

const discovered = await createBuilder(testRoot).discoverEntriesPublic(
[entryFile],
join(testRoot, 'out')
);

expect(discovered.discoveredSerdeFiles).toEqual(new Set());
});

it("does not treat the builder's own serde checker as a serde file", async () => {
// The checker's error messages quote `static [WORKFLOW_SERIALIZE](...)`.
// It is reachable from any project whose config file imports a framework
// integration, and bundling it pulls builder-only dependencies (such as
// the `builtin-modules` JSON import) into every step bundle.
const entryFile = join(testRoot, 'src', 'entry.ts');
const checkerFile = join(testRoot, 'src', 'serde-checker.ts');

writeFile(entryFile, `import './serde-checker';\n`);
writeFile(
checkerFile,
readFileSync(new URL('../src/serde-checker.ts', import.meta.url), 'utf-8')
);

const discovered = await createBuilder(testRoot).discoverEntriesPublic(
[entryFile],
join(testRoot, 'out')
);

expect(discovered.discoveredSerdeFiles).toEqual(new Set());
});

it('relativizes nested package step registration imports', () => {
const routeDir = join(testRoot, 'app', '.well-known', 'workflow', 'v1');
const directPackageFile = join(
Expand Down
34 changes: 23 additions & 11 deletions packages/builders/src/fast-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import enhancedResolveOriginal from 'enhanced-resolve';
import { findUp } from 'find-up';
import JSON5 from 'json5';
import { importParents } from './discover-entries-esbuild-plugin.js';
import { detectWorkflowPatterns } from './transform-utils.js';
import {
detectWorkflowPatterns,
stripCommentsFromSource,
} from './transform-utils.js';

const FAST_DISCOVERY_SOURCE_EXTENSIONS = [
'.ts',
Expand Down Expand Up @@ -274,35 +277,44 @@ function hasWorkflowDependency(dependencies: unknown): boolean {
);
}

function stripComments(source: string): string {
return source
.replace(/\/\*[\s\S]*?\*\//g, '')
.replace(/(^|[^:])\/\/.*$/gm, '$1');
}
// Matched against source with quoted strings masked, so the symbol name is
// blank here and is checked against the original source at the same offsets.
const MASKED_SYMBOL_FOR_SERDE_METHOD =
/static\s+\[\s*Symbol\.for\s*\(\s*(['"])\s*\1\s*\)\s*\]\s*\(/g;
const SYMBOL_FOR_SERDE_METHOD =
/^static\s+\[\s*Symbol\.for\s*\(\s*['"]workflow-(?:serialize|deserialize)['"]\s*\)\s*\]\s*\($/;

function hasLikelySerdeClass(source: string): boolean {
if (!source.includes('static') || !source.includes('[')) {
return false;
}

const uncommentedSource = stripComments(source);
// Class shapes are matched against code only, never string contents. The
// scanner output has the same length as the source, so offsets line up.
const codeOnlySource = stripCommentsFromSource(source, true, true);
if (
/static\s+\[\s*(?:WORKFLOW_(?:SERIALIZE|DESERIALIZE)|Symbol\.for\s*\(\s*['"]workflow-(?:serialize|deserialize)['"]\s*\))\s*\]\s*\(/.test(
uncommentedSource
/static\s+\[\s*WORKFLOW_(?:SERIALIZE|DESERIALIZE)\s*\]\s*\(/.test(
codeOnlySource
)
) {
return true;
}
for (const match of codeOnlySource.matchAll(MASKED_SYMBOL_FOR_SERDE_METHOD)) {
const original = source.slice(match.index, match.index + match[0].length);
if (SYMBOL_FOR_SERDE_METHOD.test(original)) {
return true;
}
}

if (
!/from\s+['"]@workflow\/serde['"]|require\s*\(\s*['"]@workflow\/serde['"]\s*\)/.test(
uncommentedSource
stripCommentsFromSource(source)
)
) {
return false;
}

return /static\s+\[\s*[$A-Z_a-z][$\w]*\s*\]\s*\(/.test(uncommentedSource);
return /static\s+\[\s*[$A-Z_a-z][$\w]*\s*\]\s*\(/.test(codeOnlySource);
}

async function loadTsconfigPathAliases(
Expand Down
Loading
Loading