Fix/node cjs packaging #1
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Node: Standardize dual-format packaging to match established codebase patterns
This Pull Request fixes the packaging for CJS consumers of the Node package.
Previously, the CommonJS entry failed due to a Rollup bundling configuration issue in
@herb-tools/node
. The package used separate.cts/.mts
source files, but Rollup wasn't properly bundling local dependencies for the CJS build, leaving runtime requires that failed.This change adopts the established pattern: a single
src/index.ts
source file that Rollup transforms into both ESM and CJS outputs. This matches the proven strategy used by@herb-tools/core
,@herb-tools/formatter
,@herb-tools/highlighter
, and@herb-tools/linter
.This change adopts the established pattern: a single
src/index.ts
source file that Rollup transforms into both ESM and CJS outputs. This matches the strategy used by@herb-tools/core
,@herb-tools/formatter
,@herb-tools/highlighter
, and@herb-tools/linter
.No public API changes.
Expected vs Actual
@herb-tools/node
, callawait Herb.load()
, and use the API.Reproduction (before)
Using the built artifacts, with the previous split
.mts/.cts
sources:Run:
node test-esm.mjs # ✓ Loads and runs successfully
Run:
node test-cjs.cjs # ✗ Error: Cannot find module './node-backend' (from dist/herb-node.cjs)
Even with the path changed, there'd still be the error:
Diagnosis (before)
CJS build problem:
src/index-cjs.cts
containedrequire("./node-backend.js")
dist/herb-node.cjs
expected./node-backend.js
to exist at runtime, but it wasn't emittednew Promise(...)
(Promise instance) instead of() => new Promise(...)
(Promise factory)ESM build worked because:
node-backend.ts
dependency() => new Promise(...)
Fix
Adopt the standard, project-wide packaging pattern:
src/index.ts
for both formats..cts/.mts
sources to avoid divergence and bundling issues.package.json
type entries to the standard./dist/types/index.d.ts
.This mirrors the approach in other packages and eliminates the non-working CJS path.
Files Changed
src/index-esm.mts
→src/index.ts
(renamed, now single source)src/index-cjs.cts
(deleted, was non-working code path)rollup.config.mjs
(updated to use standard pattern)package.json
(updated type declarations)