Skip to content

fix(@deepnote/convert): add dual ESM/CJS support#211

Merged
saltenasl merged 2 commits intomainfrom
ls/fix-convert-cjs
Jan 14, 2026
Merged

fix(@deepnote/convert): add dual ESM/CJS support#211
saltenasl merged 2 commits intomainfrom
ls/fix-convert-cjs

Conversation

@saltenasl
Copy link
Contributor

@saltenasl saltenasl commented Jan 14, 2026 •
edited by coderabbitai bot
Loading

Summary

  • Add CommonJS output to @deepnote/convert to support CJS consumers
  • Align package exports configuration with other packages in the monorepo (@deepnote/blocks, @deepnote/database-integrations, @deepnote/reactivity)

Problem

The package was ESM-only, causing import errors in CommonJS environments:

SyntaxError: Cannot use import statement outside a module

Solution

  • Updated build script to output both ESM (.js) and CJS (.cjs) formats
  • Added require condition to package exports
  • Set main to CJS entry point for Node.js CJS resolution
  • Added module field for bundler ESM resolution

Summary by CodeRabbit

  • Chores
    • Updated package configuration to properly support both CommonJS and ES Module imports with explicit export mappings.
    • Enhanced type definitions accessibility in package exports.
    • Optimized build system to generate both module formats.
    • Bumped version to 2.1.1.

✏️ Tip: You can customize this high-level summary in your review settings.

@saltenasl saltenasl requested a review from a team as a code owner January 14, 2026 04:28
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 14, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

Package.json updated to support dual ESM and CJS outputs. The exports map now explicitly defines types and require entry points. The main field references the CJS output, while a new module field points to ESM. Build scripts were updated to generate both formats via tsdown. Version bumped from 2.1.0 to 2.1.1.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding dual ESM/CJS support to the package, which directly addresses the core modification in package.json exports and build configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e457524 and d89bccd.

📒 Files selected for processing (1)
  • packages/convert/package.json
🔇 Additional comments (3)
packages/convert/package.json (3)

13-21: Exports structure looks correct.

Order (types → import → require), CJS via .cjs extension with "type": "module", and bundler-friendly module field all align with best practices.

33-35: Build scripts updated correctly.

Dual --format esm --format cjs with --dts covers both output targets.

37-44: No issue — chalk and ora are isolated to the CLI.

The ESM-only dependencies are only imported in src/cli.ts, not src/index.ts. The main library entry point (src/index.ts) contains only re-exports and has no direct dependency on these packages. CJS consumers using require() will receive the CJS build of src/index.ts without chalk or ora, and the CLI runs in ESM mode via dist/bin.js, so there's no runtime breakage.

Likely an incorrect or invalid review comment.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

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

@codecov
Copy link

codecov bot commented Jan 14, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.57%. Comparing base (e457524) to head (d89bccd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #211   +/-   ##
=======================================
  Coverage   91.57%   91.57%           
=======================================
  Files          41       41           
  Lines        2041     2041           
  Branches      652      652           
=======================================
  Hits         1869     1869           
  Misses        172      172

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@saltenasl saltenasl merged commit dec9119 into main Jan 14, 2026
20 checks passed
@saltenasl saltenasl deleted the ls/fix-convert-cjs branch January 14, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@saltenasl