Skip to content

build: add dual package support for ESM and CommonJS#140

Open
tommyo wants to merge 1 commit intogrimmdude:masterfrom
tommyo:master
Open

build: add dual package support for ESM and CommonJS#140
tommyo wants to merge 1 commit intogrimmdude:masterfrom
tommyo:master

Conversation

@tommyo
Copy link

@tommyo tommyo commented Feb 1, 2026

  • Update package.json with dual exports configuration (ESM/CJS)
  • Simplify rollup config to output both module formats
  • Remove browser-specific build configuration
  • Update TypeScript config with ES2018 target and ESNext module
  • Standardize code formatting (spacing in imports/types)
  • Bump version to 3.1.2

This change enables the package to be consumed by both ESM and CommonJS projects through conditional exports, improving compatibility across different module systems.

- Update package.json with dual exports configuration (ESM/CJS)
- Simplify rollup config to output both module formats
- Remove browser-specific build configuration
- Update TypeScript config with ES2018 target and ESNext module
- Standardize code formatting (spacing in imports/types)
- Bump version to 3.1.2

This change enables the package to be consumed by both ESM and CommonJS projects through conditional exports, improving compatibility across different module systems.
Copy link
Owner

@grimmdude grimmdude left a comment

Choose a reason for hiding this comment

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

Thanks for adding dual ESM/CJS package support — the package.json exports field looks correctly structured. A few issues I'd like addressed before merging:

1. Breaking type change to NoteOnEvent.pitch

NoteOnEvent is a public export, and the pitch type has been narrowed from string|string[]|number|number[] to string|number. External consumers who construct NoteOnEvent directly with array pitches will get a type error after this change. If this narrowing is intentional it should be split into a separate PR with an appropriate version bump and migration note — not bundled into a patch release.

2. Browser/IIFE build silently removed

The entire browser build (browser/midiwriter.js, build/index.browser.js) and @rollup/plugin-replace have been removed. This is a breaking change for anyone consuming the library via <script> tags. The PR description mentions it but this warrants more prominent treatment and a major version bump.

3. Version bump should reflect breaking changes

Given the type narrowing (#1) and browser build removal (#2), a patch bump (3.1.13.1.2) understates the scope of these changes. This should be at least a minor bump, or major if the browser build removal is considered part of the public API.

4. ESM output missing exports field in Rollup config

The CJS output specifies exports: 'default' but the ESM output has no exports field. Since main.ts uses a default export, this should be explicitly set for consistency and to avoid ambiguity. Please verify the test suite passes against both output formats.

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.

2 participants