ably-js icon indicating copy to clipboard operation
ably-js copied to clipboard

Fix named exports not working for plugins in Node.js ESM

Open VeskeR opened this issue 2 months ago • 1 comments

The issue occurs in Node.js with ESM resolution because the plugin modules are built only as UMD. As a result, Node.js with ESM cannot determine that such a UMD module has named exports.

This PR introduces separate CJS and ESM builds for plugins to resolve the issue.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added ES module builds for push and objects plugins, enabling modern import syntax support alongside CommonJS.
  • Chores

    • Updated package export configurations to support both CommonJS and ES module imports for improved module compatibility.

VeskeR avatar Oct 29 '25 09:10 VeskeR

Walkthrough

Adds ES module build configurations for push and objects plugins, generates .mjs outputs alongside existing CommonJS builds, updates package.json export entries to differentiate ESM and CommonJS paths, and modifies TypeScript type definitions to use ES module default exports.

Changes

Cohort / File(s) Summary
Build Configuration
Gruntfile.js, grunt/esbuild/build.js
Introduces two new ESM build configurations (pushPluginEsmConfig, objectsPluginEsmConfig) with esm format and .mjs output paths; integrates these into Grunt build tasks via Promise.all for parallel execution.
TypeScript Type Definitions
push.d.ts, objects.d.ts
Updates export syntax from CommonJS-style export = to ES module default exports (export default).
Package Exports
package.json
Adds dual export entries for push and objects: "import" field points to .mjs files, "require" field points to .js files, enabling conditional exports for ESM and CommonJS consumers.

Sequence Diagram

sequenceDiagram
    participant Build as Build System
    participant ESM as ESM Config
    participant CJS as CJS Config
    participant Dist as Distribution
    
    Build->>ESM: Build push/objects.mjs
    Build->>CJS: Build push/objects.js (existing)
    ESM->>Dist: Generate .mjs artifacts
    CJS->>Dist: Generate .js artifacts
    
    Note over Dist: package.json routes imports
    rect rgb(200, 240, 200)
    Dist-->>ESM: "import" → .mjs
    end
    rect rgb(240, 200, 200)
    Dist-->>CJS: "require" → .js
    end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Changes follow a consistent pattern of adding ESM support across multiple components
  • No complex logic, control flow, or error handling modifications
  • Updates are primarily configuration and declaration changes with straightforward syntax replacements
  • Package.json export changes are standard conditional export setup

Poem

🐰 A builder's gift, both old and new, CommonJS stays, while modules brew, ESM defaults, .mjs files bloom, Dual paths light every room! 📦✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Fix named exports not working for plugins in Node.js ESM" is fully related to the changeset and clearly summarizes the primary issue being addressed. The PR objectives confirm that the core problem is that Node.js ESM resolution fails to recognize named exports from plugin modules because they were only built as UMD. The changes throughout the PR—adding separate ESM builds for plugins, updating package.json exports to point to .mjs files, and converting TypeScript declarations to ES module syntax—all directly support fixing this named exports issue. The title is specific, concise, and conveys meaningful information about the problem being solved without unnecessary noise.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch fix/plugin-named-exports

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 and usage tips.

coderabbitai[bot] avatar Oct 29 '25 09:10 coderabbitai[bot]

Superseded by https://github.com/ably/ably-js/pull/2131. This PR introduced an incomplete solution which caused "Incorrect default export" error from arethetypeswrong check. https://github.com/ably/ably-js/pull/2131 resolves that but introduces a breaking change.

VeskeR avatar Dec 18 '25 08:12 VeskeR