Skip to content

fix(package): every entry point loads with require() - #29

Merged
Makisuo merged 1 commit into
mainfrom
fix/exports-default-condition
Oct 14, 2026
Merged

Makisuo merged 1 commit into
mainfrom
fix/exports-default-condition

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

Each entry in exports now has a default condition beside import, pointing at the same .mjs. CommonJS code can require() every entry point.

Why

The export map offered only import. Node's require() matches require, node and default, never import, so every CommonJS caller failed with ERR_PACKAGE_PATH_NOT_EXPORTED. That includes CommonJS builds of TypeScript projects, which compile import to require.

The modules themselves were never the problem. They have no top-level await, so Node 20.19+ and 22.12+ load them synchronously through require(esm). That is already how such callers load Effect 4, which is also ESM-only. Because default points at the same file, a CommonJS caller and an ES module caller load one module instance and share one Effect.

Found in superwall/paywall-next#4365, which had to carry a pnpm patch for this. @superwall/api ships a CommonJS build and imports a query module that uses the builder. The patch can be dropped once this is released.

Changes

  • package.json: default added to all 12 entries.
  • tests/package-consumer.cjs and scripts/check-package.ts: test:package now runs a CommonJS consumer against the packed tarball.
    • It require()s every entry, reading the list from the installed export map so that entries added later are covered.
    • It then compiles and decodes a query, to show that a CommonJS caller's effect works with the library.
  • README and getting-started note CommonJS support and the Node version floor.
  • A patch changeset.

Testing

  • bun run test:package passes (CommonJS consumer ok (12 entries)).
    • With the old export map it fails with ERR_PACKAGE_PATH_NOT_EXPORTED, so the new check catches a regression.
  • bun run typecheck passes.
  • bun run test passes: 684 tests, plus the doc citation, export catalog and doc example checks. The 204 skipped tests are the live-ClickHouse tests, which need an endpoint.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Summary by CodeRabbit

  • New Features
    • CommonJS applications can load package entry points with require() on Node.js 20.19+ or 22.12+.
    • Package documentation now describes the supported CommonJS loading versions.
  • Tests
    • Added a check that verifies every published entry point loads through CommonJS and supports a ClickHouse query returning a maximum uint64 value.

The export map offered only an `import` condition, so CommonJS callers,
including CommonJS builds of TypeScript projects, failed with
ERR_PACKAGE_PATH_NOT_EXPORTED. The modules have no top-level await and
load fine through Node's require(esm) (20.19+, 22.12+), which is how
those callers already load Effect 4. Each entry now also has a `default`
condition pointing at the same .mjs, so CommonJS and ES module callers
load one module and share one Effect.

The package-consumer check now require()s every entry listed in the
packed tarball's export map and compiles and decodes a query from
CommonJS. With the old export map it fails with the original error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@maple-review-bot

maple-review-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 8/10 · likely safe to merge
Verified Node's condition matching and require(esm) on a synthetic export map; the CJS consumer's require("effect") line is the one thing I could not exercise here.
quality 100/100 · no findings · tests covered · risk high

Adds a default condition beside import on all 12 export entries so CommonJS callers can require() the ESM entries, and extends the tarball check with a CommonJS consumer. Contained and safe to merge.

  • All 12 exports entries gain default pointing at the same .mjs as import
  • tests/package-consumer.cjs requires every entry from the installed export map
  • check-package.ts runs the CommonJS consumer against the packed tarball
  • README and getting-started document the Node 20.19+/22.12+ floor
What was checked
  • require() skips import and matches default; ran a synthetic package with the same map and it resolved dist/clickhouse.mjs
  • All 12 entries list default last; no entry and no types entry was missed (package.json:36-95)
  • The CJS consumer fails with the old map, so the new check catches the regression

5bb8bb2 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 14da5390-835e-470b-b5e2-c504f4f64a66
📥 Commits

Reviewing files that changed from the base of the PR and between 0623054 and 5bb8bb2.

📒 Files selected for processing: 6
  • .changeset/commonjs-require.md
  • README.md
  • docs/getting-started.md
  • package.json
  • scripts/check-package.ts
  • tests/package-consumer.cjs
 _____________________________________________________________________________________________________________________________________
< Prototype to learn. Prototyping is a learning experience. Its value lies not in the code you produce, but in the lessons you learn. >
 -------------------------------------------------------------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Makisuo
Makisuo merged commit 00e241c into main Oct 14, 2026
3 of 4 checks passed
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.

1 participant