Skip to content

[codex] fix node workflow dynamic require builds#3

Merged
mihaipxm merged 1 commit into
mainfrom
codex/fix-node-workflow-dynamic-require
May 14, 2026
Merged

[codex] fix node workflow dynamic require builds#3
mihaipxm merged 1 commit into
mainfrom
codex/fix-node-workflow-dynamic-require

Conversation

@mihaipxm
Copy link
Copy Markdown
Contributor

Summary

Fixes #2.

This updates node-platform workflow builds so bundled CommonJS dependencies that dynamically require Node built-ins, such as stream, can be loaded during manifest extraction and by the self-hosted Node runner.

Root Cause

The CLI builds workflow entries as ESM and immediately imports bundle.mjs to collect registered workflows. When esbuild preserves a CommonJS dynamic require helper in a node-platform ESM bundle, Node does not provide an ambient require, so manifest extraction fails even though the dependency is valid for Node.

Changes

  • Adds a node-only esbuild banner that defines require with createRequire(import.meta.url).
  • Adds a regression test using a real bundled CJS dependency that dynamically requires stream.
  • Verifies neutral/browser bundles do not receive the Node require shim.
  • Documents that node workflow bundles are loaded through Node's file-backed ESM loader contract.
  • Adds a patch changeset for @voyantjs/cli.

Validation

  • pnpm --filter @voyantjs/cli check-types
  • pnpm lint
  • pnpm --filter @voyantjs/cli test

@mihaipxm mihaipxm marked this pull request as ready for review May 14, 2026 10:27
@mihaipxm mihaipxm merged commit 912023f into main May 14, 2026
3 checks passed
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f47c30cc46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


const NODE_REQUIRE_BANNER = [
`import { createRequire as __voyantCreateRequire } from "node:module";`,
`const require = __voyantCreateRequire(import.meta.url);`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid declaring a fixed top-level require

For --platform node this banner is prepended verbatim to every bundled ESM file, so any workflow entry that already declares a top-level require (for example the standard Node ESM pattern const require = createRequire(import.meta.url)) will produce a bundle with two top-level require declarations and fail to import during manifest extraction with an identifier redeclaration error. Because esbuild does not parse banner text for name collision avoidance, use an injected/renamed shim or a less collision-prone helper instead of reserving require globally in the bundle.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node workflow build fails on dynamic require of Node built-ins

1 participant