feat(cli, builders): support WORKFLOW_EXTERNAL_PACKAGES env var#1292
feat(cli, builders): support WORKFLOW_EXTERNAL_PACKAGES env var#1292jackmazac wants to merge 2 commits into
Conversation
Parse a comma-separated WORKFLOW_EXTERNAL_PACKAGES environment variable in the CLI config and pass it through to the final workflow bundle's esbuild external array. The externalPackages config field already existed in the type definition and was wired into the discovery and steps bundles, but was missing from the final workflow bundle and had no way to be set via the CLI. This allows users with native binary dependencies (e.g. pg, pg-native) to externalize them from workflow bundles without patching node_modules. Made-with: Cursor
🦋 Changeset detectedLatest commit: c38c040 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
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 |
|
@jackmazac is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
Review
Appreciate the contribution, and the underlying pain is real — there is no way today to feed `externalPackages` to the CLI. But I think this PR is addressing the wrong layer and the `base-builder.ts` change is a no-op that won't fix the reported problem. See inline comments for specifics; the big-picture argument:
1. The bundle this PR modifies doesn't actually import the packages you're externalizing
`base-builder.ts` line ~956 (where the `external` array is) is the final/outer workflow bundle — the wrapper whose entire source is:
```js
import { workflowEntrypoint } from 'workflow/runtime';
const workflowCode = `…huge string of the intermediate bundle…`;
export const POST = workflowEntrypoint(workflowCode);
```
The intermediate bundle is already a pre-bundled string literal by the time this esbuild pass runs. Adding `pg` / `pg-native` / `better-sqlite3` to this outer `external` array has no effect because nothing in the outer source references them — they're inside a string, not an import. The outer bundle only imports `workflow/runtime`.
To verify: run `workflow build` with `WORKFLOW_EXTERNAL_PACKAGES=pg` on your repro and diff `flow.js` byte-for-byte against a build without it. They'll be identical.
2. The bundle where `pg` actually lives — the intermediate workflow bundle — cannot be externalized
Lines 803-810 of `base-builder.ts` (the block above the bundle this PR modifies) contain an explicit comment explaining exactly why:
NOTE: We intentionally do NOT use the external option here for workflow bundles. When packages are marked external with format: 'cjs', esbuild generates require() calls. However, the workflow VM (vm.runInContext) does not have require() defined — it only provides module.exports and exports. External packages would fail at runtime with: ReferenceError: require is not defined
That block builds the bundle that runs inside the VM. Externalization there would produce `require('pg')` calls and fail at runtime.
3. You're likely seeing a dead-code-elimination / mode-split issue, not an externalization gap
If `pg` or `better-sqlite3` is ending up inside the VM-executed intermediate bundle, the root cause is almost certainly one of:
- A step function that imports `pg` isn't being correctly stripped by the SWC `mode: 'workflow'` transform — the step body should be replaced with a stub in the workflow bundle, so its imports never reach esbuild. If `pg` is reaching esbuild in the workflow bundle, the SWC plugin likely missed a side-effect import or failed to strip the step body.
- A workflow file (not a step) is importing `pg` directly — which wouldn't be valid user code (workflows shouldn't do I/O), but could happen via a transitive shared module that has top-level side effects.
Either of these would manifest as "pg is in the bundle" and feel like an externalization problem to the user. The right fix is to find the import chain that's pulling `pg` into the workflow mode bundle and either (a) fix the SWC plugin to DCE it properly, or (b) restructure user code so `pg` isn't imported at module scope outside of a step.
Could you share the actual symptom you saw? A build-time warning, a runtime error with a stack trace, or a snippet of `flow.js` showing where `pg` ends up? That would let us target the actual bug.
4. If externalization for the CLI is genuinely needed, it should piggyback on the existing framework config
The Next.js integration already does this correctly at `packages/next/src/index.ts:138-146`:
```ts
externalPackages: [
'server-only',
'client-only',
...(nextConfig.serverExternalPackages || []),
],
```
It reads from Next's own `serverExternalPackages` config rather than introducing a workflow-specific mechanism. For the CLI, the equivalent would be reading from a `workflow.config.ts` or similar — which the CLI currently doesn't have. A bespoke env var (`WORKFLOW_EXTERNAL_PACKAGES`) is the wrong shape: it's workflow-SDK-specific, untyped, awkward for more than a few packages, and doesn't compose with other build tooling. And crucially, it should only ever affect the steps bundle (real Node runtime with `require`), not the workflow bundle.
Summary
- The `base-builder.ts` change is effectively a no-op (nothing to externalize in the outer bundle).
- Externalizing the intermediate/VM bundle would be actively broken (deliberately prevented by the existing code).
- The user's reported symptom is almost certainly a DCE / mode-split bug, not a missing config option.
- If/when we do add externalization to the CLI, it should (a) be a structured config, not an env var, and (b) only apply to the steps bundle.
I'd suggest closing this PR and filing an issue with a repro of the `pg` / native-binary bundling problem so we can diagnose the actual root cause.
| minify: false, | ||
| external: ['@aws-sdk/credential-provider-web-identity'], | ||
| external: [ | ||
| '@aws-sdk/credential-provider-web-identity', |
There was a problem hiding this comment.
This change is a no-op.
This esbuild pass (lines ~934-957 in the file) bundles the outer wrapper whose source is:
import { workflowEntrypoint } from 'workflow/runtime';
const workflowCode = `…intermediate bundle as a string…`;
export const POST = workflowEntrypoint(workflowCode);The outer source doesn't reference pg, pg-native, or any user package directly — those are already bundled into the workflowCode string by the intermediate pass. Adding them to external here doesn't remove anything from the output; esbuild only externalizes what's actually referenced by the source being bundled.
| const parts = raw | ||
| .split(',') | ||
| .map((p) => p.trim()) | ||
| .filter(Boolean); |
There was a problem hiding this comment.
If we do want to plug the CLI's config gap (there is genuinely no way today to set externalPackages via the CLI), I'd suggest reading from a structured config file (workflow.config.ts or similar) rather than a bespoke env var. That:
- Matches how the Next.js integration already sources
externalPackages(fromnextConfig.serverExternalPackages— seepackages/next/src/index.ts:138). - Is typed and discoverable in editors.
- Composes with other config (dirs, observability settings, etc.) instead of requiring one env var per option.
- Survives
pnpm build/ CI / cross-platform use without shell-escaping concerns.
Even if you keep the env var as an MVP, the base-builder.ts change in this PR should be dropped because it doesn't actually externalize anything. The CLI plumbing alone would at least make externalPackages reach the steps bundle (where it's effective).
2 participants
Summary
WORKFLOW_EXTERNAL_PACKAGESenv var (comma-separated package names) in the CLI config and wire it toexternalPackageson theWorkflowConfigobject.externalPackagesspread to the final workflow bundle's esbuildexternalarray, completing coverage across all three bundle phases (discovery, steps, and workflow bundles).Motivation
The
externalPackagesfield already exists on theWorkflowConfigtype and is respected during discovery and steps bundling, but:getWorkflowConfig()never sets it — there's no way to populate it from the environment.base-builder.tsdoesn't include it in itsexternalarray.This means users with native binary dependencies (e.g.,
pg,pg-native,better-sqlite3) that can't be bundled by esbuild have no built-in way to externalize them from workflow bundles when using the CLI. The only workaround is patchingnode_modulesafter install.Changes
packages/cli/src/lib/config/workflow-config.tsparseExternalPackages()that readsWORKFLOW_EXTERNAL_PACKAGES, splits on commas, trims whitespace, and returnsstring[] | undefined.getWorkflowConfig()via the existingexternalPackagesconfig field.packages/builders/src/base-builder.tsthis.config.externalPackagesinto the final workflow bundle'sexternalarray (lines ~809-813), matching the pattern already used in the discovery and steps bundles.Usage
Test plan
workflow buildwithout the env var produces identical output (no regression)workflow buildwithWORKFLOW_EXTERNAL_PACKAGES=pg,pg-nativeexcludes those packages from the finalflow.jsbundle" pg , pg-native "→["pg", "pg-native"]Made with Cursor