Skip to content

feat(js): add a CommonJS entry point for the synchronous API - #193

Merged
konard merged 3 commits into
mainfrom
issue-189-a73113905acf
Aug 11, 2026
Merged

feat(js): add a CommonJS entry point for the synchronous API#193
konard merged 3 commits into
mainfrom
issue-189-a73113905acf

Conversation

@konard

@konard konard commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary

command-stream@0.18.0 published a single ESM specifier, so a CommonJS host could not use the synchronous API:

$ node -e 'require("command-stream")'
Error [ERR_REQUIRE_ESM]: require() of ES Module .../command-stream/src/$.mjs not supported.

await import('command-stream') works but is asynchronous by definition, which makes it unusable at a synchronous launch-time probe boundary — exactly where ProcessRunner.sync() is most useful.

This PR publishes src/$.cjs under the require export condition. It loads the existing ESM module graph through require(esm) and re-exports the callable $ with every named export attached to it.

Fixes #189

Root cause

js/package.json declared only the ESM entry:

"main": "src/$.mjs",
"exports": { ".": "./src/$.mjs" }

There is no require condition, so Node resolves require('command-stream') straight to the .mjs file. On runtimes without require(esm) that throws ERR_REQUIRE_ESM; on Node.js >= 20.19 / >= 22.12 it resolves, but to a module namespace object, which is not callable — loaded`echo hi` fails with TypeError: loaded is not a function.

Approach

src/$.cjs is a thin wrapper rather than a bundled CJS build. That keeps a single module instance, so require() and import() share the same virtualCommands registry, shell settings and cleanup state — no dual-package hazard, and no build step:

const namespace = require('./$.mjs');          // require(esm)

function $(...args) {
  return namespace.$(...args);                 // forwards, never mutates the ESM `$`
}
// every named export is then attached to `$`, plus `default`, `$` and `__esModule`

Runtimes without require(esm) support now get an actionable message instead of ERR_REQUIRE_ESM:

command-stream: require() of this package needs a runtime with require(esm) support - Node.js >= 20.19.0 or >= 22.12.0 (running v18.20.4). Upgrade Node.js, or load the package with await import('command-stream').

Reproduction

js/experiments/repro-189-commonjs-require.mjs installs the package into a throwaway sandbox and reports what a CommonJS host observes:

$ node js/experiments/repro-189-commonjs-require.mjs

Before (Node.js v20.20.2, package.json without the require condition):

typeof require result: object
callable as a tagged template: false
TypeError: loaded is not a function
exit status: 1

After:

typeof require result: function
callable as a tagged template: true
sync probe: "sync-probe\n" code: 0
exit status: 0

Usage

const $ = require('command-stream');
const { ProcessRunner, shell } = require('command-stream');

const probe = $({ mirror: false, capture: true })`git --version`.sync();
if (probe.code === 0) {
  console.log(probe.stdout.trim());
}

js/examples/commonjs-launch-probe.cjs demonstrates the issue's real-world use case (synchronous launch-time availability probes) and runs under both node and bun.

Tests

  • js/tests/commonjs-entry.test.mjs — 9 tests: manifest wiring, in-process require() shape (callable $, default, __esModule, named exports), a real .sync() call through the CJS entry, require()/import() returning the identical ProcessRunner/shell objects, and that the ESM $ is left unpolluted. Three of them spawn a real node process resolving the bare specifier command-stream from a sandbox.
  • js/tests/node-commonjs-entry.mjsnode --test suite so the CI Node.js matrix (20, 22, 24) exercises require('command-stream') end to end; skips itself on runtimes without require(esm).
  • js/tests/commonjs-sandbox.mjs — shared sandbox helper used by both suites and the experiment.
  • .github/workflows/js.yml — the Node.js compatibility job now loads src/$.cjs with require() and runs the new node --test suite.
$ bun test js/tests/commonjs-entry.test.mjs
 9 pass, 0 fail, 33 expect() calls

$ node --test js/tests/node-commonjs-entry.mjs
 pass 1, fail 0

Full suite: 808 pass, 5 skip, 0 fail across 59 files. eslint ., prettier --check . and jscpd . are clean.

CI on this branch is green after merging main (0.18.2). The Node.js matrix
confirms the entry point on every supported version:

Job Runtime Result
node on ubuntu-latest v20.20.2 CommonJS entry loads successfully in Node.js 20
node on ubuntu-latest v22.23.1 CommonJS entry loads successfully in Node.js 22
node on ubuntu-latest v24.18.0 CommonJS entry loads successfully in Node.js 24

The new node --test suite passes on all three.

Merge with main (0.18.2)

main advanced while this PR was open, which produced three conflicts:

  • js/package.jsonmain added a ./process-runner subpath export (feat(js): expose lightweight ProcessRunner entry #194).
    The merged exports map keeps that subpath alongside the new require /
    import conditions for .. The subpath deliberately keeps its plain string
    target: it exposes only the named export ProcessRunner, which require(esm)
    already returns directly, so it needs no CommonJS wrapper —
    require('command-stream/process-runner').ProcessRunner is a function.
  • js/README.md — both additions kept: the upstream "Lightweight
    ProcessRunner entry point" subsection and this PR's "Module Formats" section.
  • .gitkeep — accepted the upstream deletion (100be07).

src/$.cjs enumerates the ESM namespace at load time rather than listing exports,
so the exports main added to $.mjs are re-exported automatically with no change
to the wrapper.

Changes

File Change
js/src/$.cjs New CommonJS entry point
js/package.json main / module / exports conditions for require and import
js/README.md New "Module Formats (ESM and CommonJS)" section + a CJS synchronous example
js/eslint.config.js Lint **/*.cjs as CommonJS scripts
js/.changeset/commonjs-entry-point.md minor release

No existing behaviour changes: the ESM entry point, its exports and every existing test are untouched.

Language parity

Labelled parity-exempt: this is a JavaScript module-resolution change (package.json export conditions and a CommonJS entry point). CommonJS has no counterpart in the Rust crate, so there is no equivalent change to make under rust/src/**. No runtime behaviour of the library itself changed.

Adding .gitkeep for PR creation (default mode).
This file will be removed when the task is complete.

Issue: #189
@konard konard self-assigned this Aug 11, 2026
Issue #189: the package exported only ./src/$.mjs, so a CommonJS host could
reach the library only through `await import('command-stream')`. That is
asynchronous by definition, which made the synchronous ProcessRunner.sync()
API unusable at a synchronous launch-time probe boundary. On runtimes without
require(esm) the require() call failed with ERR_REQUIRE_ESM; on newer Node.js
it resolved to a module namespace object that is not callable as `$`.

src/$.cjs is now published under the "require" export condition. It loads the
same ESM module graph via require(esm), so require() and import() share one
instance (no dual-package hazard) and exports the `$` tagged template with
every named export attached to it. Runtimes without require(esm) support get
an actionable error instead of ERR_REQUIRE_ESM.

- src/$.cjs: CommonJS entry point
- package.json: main/module/exports conditions for require and import
- tests/commonjs-entry.test.mjs: Bun suite (shape, sync(), shared instance)
- tests/node-commonjs-entry.mjs: node --test suite for the CI Node matrix
- tests/commonjs-sandbox.mjs: shared bare-specifier sandbox helper
- experiments/repro-189-commonjs-require.mjs: issue reproduction harness
- examples/commonjs-launch-probe.cjs: synchronous launch-time probe example
- eslint.config.js: lint .cjs files as CommonJS scripts
- js.yml: exercise the CommonJS entry on Node.js 20, 22 and 24
- README.md: document both module formats
@konard konard changed the title [WIP] Provide a synchronous CommonJS entry point for ProcessRunner.sync feat(js): add a CommonJS entry point for the synchronous API Aug 11, 2026
@konard konard added the parity-exempt Change is intentionally single-language; skip JS/Rust parity check label Aug 11, 2026
@konard
konard marked this pull request as ready for review August 11, 2026 04:28
@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

Working session summary

Done. PR #193 is pushed, updated and marked ready for review: #193

Root cause

js/package.json declared only the ESM specifier — "main": "src/$.mjs", "exports": { ".": "./src/$.mjs" }. With no require condition, Node resolves require('command-stream') straight to the .mjs file: ERR_REQUIRE_ESM on runtimes without require(esm), and on Node ≥ 20.19 a module namespace object that isn't callable as $.

Fix

js/src/$.cjs is now published under the require condition. It loads the same ESM graph through require(esm), so require() and import() share one instance (no dual-package hazard — verified by asserting require().ProcessRunner === (await import()).ProcessRunner), and exports a forwarding $ with every named export attached, so the ESM $ is never mutated. Runtimes without require(esm) get an actionable error instead of ERR_REQUIRE_ESM.

Evidence

js/experiments/repro-189-commonjs-require.mjs (sandbox resolving the bare specifier), Node v20.20.2 — before: typeof require result: object, callable as a tagged template: false, TypeError: loaded is not a function, exit 1. After: typeof … function, callable … true, sync probe: "sync-probe\n" code: 0, exit 0.

Verification

  • bun test js/tests/commonjs-entry.test.mjs → 9 pass / 0 fail
  • node --test js/tests/node-commonjs-entry.mjs → 1 pass
  • Full suite bun test js/tests/804 pass, 5 skip, 0 fail across 57 files (the 28 earlier failures were a missing jq in this container; they pass after installing it)
  • eslint ., prettier --check ., jscpd . clean
  • CI green: lint, changeset check, Bun on ubuntu/macos/windows, and the Node matrix — raw job logs confirm "CommonJS entry loads successfully in Node.js 20 / 22 / 24" and the new node --test suite passing on all three.

One CI note worth flagging: the "JS/Rust source parity" job failed because js/src/** changed without rust/src/**. CommonJS has no counterpart in the Rust crate, so I added the repo's documented escape hatch, the parity-exempt label (used the same way on merged PRs #181/#184/#186), and explained it in the PR body; the job now reports skipping. If a maintainer would rather see a Rust-side change, that decision is theirs to reverse.

Also included: js/examples/commonjs-launch-probe.cjs (the issue's launch-probe use case, runs under both node and bun), a README "Module Formats" section, an ESLint block for **/*.cjs, and a minor changeset.


This summary was automatically extracted from the AI working session output.

@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost: $7.682410

📊 Context and tokens usage:

Claude Opus 5: (2 sub-sessions)

  1. 115.6K / 1M (12%) input tokens, 45.7K / 128K (36%) output tokens
  2. 70.8K / 1M (7%) input tokens, 10.3K / 128K (8%) output tokens

Total: (2.4K new + 151.6K cache writes + 9.0M cache reads) input tokens, 65.2K output tokens, $7.682410 cost

🤖 Models used:

  • Tool: Anthropic Claude Code
  • Requested: opus (claude-opus-5)
  • Thinking level: high (~23999 tokens)
  • Model: Claude Opus 5 (claude-opus-5)

📎 Log file uploaded as Gist (3317KB)


Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

🔄 Auto-restart 1/5

Reason: CI failures detected; Merge conflicts

Starting new session to address the issues.


Auto-restart-until-mergeable mode is active. This run will stop after 5 restart iterations in total.

@konard
konard marked this pull request as draft August 11, 2026 10:24
@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

⏳ Usage Limit Reached

The automated solution draft was interrupted because the Anthropic Claude Code usage limit was reached.

📊 Limit Information

  • Tool: Anthropic Claude Code
  • Limit Type: Usage limit exceeded
  • Reset Time: in 34m (Aug 11, 11:00 AM UTC)
  • Session ID: 71451a5f-fdb5-4ad3-83d9-9cec784ae0d8

🔄 How to Continue

Auto-restart is enabled. The session will automatically restart (fresh start) when the limit resets.

🤖 Models used:

  • Tool: Anthropic Claude Code
  • Requested: opus (claude-opus-5)
  • Model: Claude Opus 5 (claude-opus-5)

📎 Execution log uploaded as Gist (4957KB)


This session was interrupted due to usage limits. The session will automatically restart when the limit resets.

Resolves three conflicts against main at 0.18.2:

- js/package.json: combine the new "require"/"import" export conditions
  for "." with the "./process-runner" subpath added upstream in #194.
  The subpath needs no CommonJS wrapper - it exposes only the named
  export ProcessRunner, which require(esm) already returns directly.
- js/README.md: keep both additions - the upstream "Lightweight
  ProcessRunner entry point" subsection and the new "Module Formats
  (ESM and CommonJS)" section.
- .gitkeep: accept the upstream deletion from 100be07.
@konard
konard marked this pull request as ready for review August 11, 2026 11:25
@konard
konard merged commit 3b21e76 into main Aug 11, 2026
12 checks passed
@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

🎉 Auto-merged

This pull request has been automatically merged by hive-mind.

  • All CI checks have passed

Auto-merged by hive-mind with --auto-merge flag

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

Labels

parity-exempt Change is intentionally single-language; skip JS/Rust parity check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide a synchronous CommonJS entry point for ProcessRunner.sync

1 participant