fix(builders): avoid false-positive node-module errors for step-only usage in shared modules (vercel#1821)

The workflow-node-module-error plugin records violations in onResolve, which
runs before esbuild's tree-shaker. A shared module with both a workflow-safe
export and a step-only export that imports node:* would be flagged even when
the workflow never reached the step-only export.

Mark node:/bun: imports as sideEffects: false so esbuild can tree-shake them
when unused in surviving code, enable metafile, and filter recorded violations
in onEnd against outputs[].imports so only builtins that actually survive in
the emitted bundle are reported.

Fixes vercel#1817

Author: TooTallNate

.changeset/node-module-error-cross-file-dce.md

@@ -0,0 +1,5 @@
+---
+"@workflow/builders": patch
+---
+
+Fix false-positive `workflow-node-module-error` for step-only Node.js usage in shared modules

packages/builders/src/node-module-esbuild-plugin.test.ts

@@ -378,6 +378,116 @@ describe('workflow-node-module-error plugin', () => {
     }
   });
 
+  it('should not error when a shared module exposes a workflow-safe export alongside an unused Node.js-using export', async () => {
+    // Repro for https://github.com/vercel/workflow/issues/1817
+    //
+    // When a shared module has both a workflow-safe export and a Node.js-using
+    // export, and the workflow bundle only uses the workflow-safe one, esbuild
+    // should be able to tree-shake the Node.js-using export (and its imports)
+    // and the plugin should not emit a false-positive violation.
+    const tempDir = mkdtempSync(join(realTmpdir, 'node-module-plugin-test-'));
+    const sharedFile = join(tempDir, 'shared.ts');
+    const entryFile = join(tempDir, 'workflow.ts');
+
+    writeFileSync(
+      sharedFile,
+      `
+      import { readFile } from "node:fs/promises";
+      import path from "node:path";
+
+      export const isSupportedValue = (value) => value === "ok";
+
+      export async function readFixtureFile() {
+        return readFile(path.join(process.cwd(), "fixture.txt"), "utf8");
+      }
+      `
+    );
+    writeFileSync(
+      entryFile,
+      `
+      import { isSupportedValue } from "./shared.js";
+      export function workflow() {
+        return isSupportedValue("ok");
+      }
+      `
+    );
+
+    try {
+      const result = await esbuild.build({
+        entryPoints: [entryFile],
+        bundle: true,
+        write: false,
+        platform: 'neutral',
+        logLevel: 'silent',
+        plugins: [createNodeModuleErrorPlugin()],
+      });
+      expect(result.errors).toHaveLength(0);
+    } finally {
+      rmSync(tempDir, { recursive: true, force: true });
+    }
+  });
+
+  it("should still error when a shared module's Node.js-using export is actually referenced", async () => {
+    // Counterpart to the previous test: when the workflow really does pull in
+    // the Node.js-using export (directly or transitively), the plugin must
+    // still emit the violation.
+    const tempDir = mkdtempSync(join(realTmpdir, 'node-module-plugin-test-'));
+    const sharedFile = join(tempDir, 'shared.ts');
+    const entryFile = join(tempDir, 'workflow.ts');
+    const relativeShared = relative(process.cwd(), sharedFile);
+
+    writeFileSync(
+      sharedFile,
+      `
+      import { readFile } from "node:fs/promises";
+      import path from "node:path";
+
+      export const isSupportedValue = (value) => value === "ok";
+
+      export async function readFixtureFile() {
+        return readFile(path.join(process.cwd(), "fixture.txt"), "utf8");
+      }
+      `
+    );
+    writeFileSync(
+      entryFile,
+      `
+      import { isSupportedValue, readFixtureFile } from "./shared.js";
+      export async function workflow() {
+        if (!isSupportedValue("ok")) throw new Error("nope");
+        return readFixtureFile();
+      }
+      `
+    );
+
+    try {
+      await esbuild.build({
+        entryPoints: [entryFile],
+        bundle: true,
+        write: false,
+        platform: 'neutral',
+        logLevel: 'silent',
+        plugins: [createNodeModuleErrorPlugin()],
+      });
+      throw new Error('Expected build to fail');
+    } catch (error: any) {
+      if (error && typeof error === 'object' && 'errors' in error) {
+        const failure = error as esbuild.BuildFailure;
+        const paths = failure.errors.map((e) => e.text);
+        expect(paths.some((t) => t.includes('"node:fs/promises"'))).toBe(true);
+        expect(paths.some((t) => t.includes('"node:path"'))).toBe(true);
+        // Violations should be attributed to the shared module, not the entry.
+        for (const err of failure.errors) {
+          expect(err.location?.file).toBe(relativeShared);
+        }
+      } else {
+        throw error;
+      }
+    } finally {
+      rmSync(tempDir, { recursive: true, force: true });
+    }
+  });
+
   it('should not point to JSDoc comments when finding usage', async () => {
     const testCode = `
       import { Writable } from "stream";

packages/builders/src/node-module-esbuild-plugin.ts

@@ -305,6 +305,15 @@ export function createNodeModuleErrorPlugin(): esbuild.Plugin {
       const packageViolations: PackageViolation[] = [];
       const seenViolations = new Set<string>();
 
+      // Enable the esbuild metafile so we can inspect which Node.js / Bun
+      // built-in imports actually survive tree-shaking. We need this to
+      // suppress false positives where a shared module has a workflow-safe
+      // export alongside a step-only export that references Node.js builtins
+      // — esbuild's tree-shaker can drop the step-only branch, but `onResolve`
+      // fires before tree-shaking, so we defer the final error decision to
+      // `onEnd` (see the metafile-based filter below).
+      build.initialOptions.metafile = true;
+
       // Track ALL import relationships to build the dependency graph.
       // This is necessary to trace transitive dependencies back to user code.
       // Performance impact is minimal as we only store path mappings.
@@ -388,9 +397,14 @@ export function createNodeModuleErrorPlugin(): esbuild.Plugin {
         const workflowFile = filteredChain[0] ?? importerPath;
 
         if (!workflowFile || workflowFile.includes('node_modules')) {
+          // Mark builtin imports as side-effect-free so esbuild can tree-shake
+          // them when they're only referenced by dead (e.g. step-only) code
+          // paths in shared modules. The onEnd filter below uses the metafile
+          // to decide which imports actually survive.
           return {
             path: args.path,
             external: true,
+            sideEffects: false,
           };
         }
 
@@ -422,39 +436,80 @@ export function createNodeModuleErrorPlugin(): esbuild.Plugin {
           }
         }
 
+        // Mark builtin imports as side-effect-free so esbuild can tree-shake
+        // them when they're only referenced by dead (e.g. step-only) code
+        // paths in shared modules. The onEnd filter below uses the metafile
+        // to decide which imports actually survive.
         return {
           path: args.path,
           external: true,
+          sideEffects: false,
         };
       });
 
-      // Report all violations at the end of the build
-      build.onEnd(() => {
-        if (packageViolations.length > 0) {
-          return {
-            errors: packageViolations.map((violation) => {
-              const isBuiltinModule = runtimeModulesRegex.test(
-                violation.packageName
-              );
-              const moduleType = getModuleTypeLabel(violation.path);
-
-              // Different messages for built-in modules vs npm packages that use them
-              const text = isBuiltinModule
-                ? `You are attempting to use "${violation.packageName}" which is a ${moduleType} module. ${moduleType} modules are not available in workflow functions.\n\nLearn more: https://workflow-sdk.dev/err/${ERROR_SLUGS.NODE_JS_MODULE_IN_WORKFLOW}`
-                : `You are attempting to use "${violation.packageName}" which depends on ${moduleType} modules. Packages that depend on ${moduleType} modules are not available in workflow functions.\n\nLearn more: https://workflow-sdk.dev/err/${ERROR_SLUGS.NODE_JS_MODULE_IN_WORKFLOW}`;
-
-              return {
-                text,
-                location: violation.location
-                  ? {
-                      ...violation.location,
-                      suggestion: 'Move this function into a step function.',
-                    }
-                  : undefined,
-              };
-            }),
-          };
+      // Report violations at the end of the build, filtered to only those
+      // whose Node.js / Bun builtin imports actually survived tree-shaking.
+      //
+      // `onResolve` fires during module loading, before esbuild's tree-shaker
+      // runs. That means a shared module whose step-only export references a
+      // builtin will record a violation even if the workflow never touches
+      // that export. By marking builtin imports as `sideEffects: false` in
+      // `onResolve` above, esbuild can drop such imports from the output.
+      // Here we consult the metafile to see which builtin imports remain
+      // in the emitted bundle and only report those.
+      build.onEnd((result) => {
+        if (packageViolations.length === 0) return;
+
+        const survivingBuiltins = new Set<string>();
+        const outputs = result.metafile?.outputs;
+        if (outputs) {
+          for (const output of Object.values(outputs)) {
+            for (const imp of output.imports) {
+              // External imports in the metafile are recorded with the path
+              // that was returned from `onResolve` (e.g. "node:fs/promises",
+              // "fs", "bun:sqlite"). Only collect the ones that match
+              // Node.js / Bun builtins.
+              if (imp.external && runtimeModulesRegex.test(imp.path)) {
+                survivingBuiltins.add(imp.path);
+              }
+            }
+          }
         }
+
+        // If the metafile is unavailable for some reason, fall back to
+        // reporting all recorded violations so we never silently suppress
+        // real ones.
+        const liveViolations = outputs
+          ? packageViolations.filter((violation) =>
+              survivingBuiltins.has(violation.path)
+            )
+          : packageViolations;
+
+        if (liveViolations.length === 0) return;
+
+        return {
+          errors: liveViolations.map((violation) => {
+            const isBuiltinModule = runtimeModulesRegex.test(
+              violation.packageName
+            );
+            const moduleType = getModuleTypeLabel(violation.path);
+
+            // Different messages for built-in modules vs npm packages that use them
+            const text = isBuiltinModule
+              ? `You are attempting to use "${violation.packageName}" which is a ${moduleType} module. ${moduleType} modules are not available in workflow functions.\n\nLearn more: https://workflow-sdk.dev/err/${ERROR_SLUGS.NODE_JS_MODULE_IN_WORKFLOW}`
+              : `You are attempting to use "${violation.packageName}" which depends on ${moduleType} modules. Packages that depend on ${moduleType} modules are not available in workflow functions.\n\nLearn more: https://workflow-sdk.dev/err/${ERROR_SLUGS.NODE_JS_MODULE_IN_WORKFLOW}`;
+
+            return {
+              text,
+              location: violation.location
+                ? {
+                    ...violation.location,
+                    suggestion: 'Move this function into a step function.',
+                  }
+                : undefined,
+            };
+          }),
+        };
       });
     },
   };