Fix named exports not working for plugins in Node.js ESM #2106
Conversation
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 commit introduces separate CJS and ESM builds for plugins to resolve the issue.
WalkthroughAdds 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
Sequence DiagramsequenceDiagram
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
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
Gruntfile.js(2 hunks)grunt/esbuild/build.js(3 hunks)objects.d.ts(1 hunks)package.json(1 hunks)push.d.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
objects.d.ts (2)
src/plugins/objects/objects.ts (1)
Objects(39-529)src/plugins/objects/index.ts (1)
Objects(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: test-npm-package
- GitHub Check: test-node (20.x)
- GitHub Check: test-node (18.x)
- GitHub Check: test-node (16.x)
- GitHub Check: test-browser (chromium)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-browser (webkit)
🔇 Additional comments (10)
Gruntfile.js (2)
128-133: ESM build correctly added to push plugin build pipeline.The addition of
esbuildConfig.pushPluginEsmConfigfollows the existing pattern and properly parallelizes the ESM build with the other push plugin build variants.
145-150: ESM build correctly added to objects plugin build pipeline.The addition of
esbuildConfig.objectsPluginEsmConfigfollows the existing pattern and maintains consistency with the push plugin changes.objects.d.ts (1)
28-28: TypeScript declaration updated consistently with push plugin.This change mirrors the push.d.ts update and correctly uses ES module default export. As with the push plugin, this is a breaking change for TypeScript users using
import Objects = require('ably/objects')syntax.grunt/esbuild/build.js (4)
65-72: Push plugin ESM configuration is correctly structured.The configuration appropriately:
- Overrides format to 'esm' for ES module output
- Removes UMD wrapper plugin (empty plugins array)
- Uses .mjs extension for the output file
- Preserves the external dependency on 'ulid'
97-104: Objects plugin ESM configuration follows the correct pattern.The configuration mirrors the push plugin ESM setup and correctly:
- Uses ESM format
- Removes UMD wrapper
- Outputs to .mjs file
- Preserves the external dependency on 'dequal'
127-127: ESM configuration properly exported.The new pushPluginEsmConfig is correctly added to module.exports, making it available for the Gruntfile.js build tasks.
131-131: ESM configuration properly exported.The new objectsPluginEsmConfig is correctly added to module.exports, maintaining consistency with the push plugin exports.
package.json (3)
29-33: Push plugin exports correctly configured for dual package support.The conditional exports properly distinguish between ESM (
import→.mjs) and CommonJS (require→.js), enabling Node.js to use the appropriate module format. The TypeScript types entry is correctly positioned for proper type resolution.
34-38: Objects plugin exports correctly configured for dual package support.The conditional exports mirror the push plugin structure and correctly route ESM and CommonJS imports to their respective build outputs.
40-49: Configuration is correct—ESM outputs will be generated.The build process is properly configured:
build:pushandbuild:objectstasks invoke esbuild with ESM configspackage.jsonexports correctly reference./build/push.mjsand./build/objects.mjsfilesarray includes"build/**"to package themThe
build/directory is gitignored (as expected for generated artifacts) and will be created during the build process locally.
| declare const Push: any; | ||
|
|
||
| export = Push; | ||
| export default Push; |
There was a problem hiding this comment.
🧩 Analysis chain
TypeScript declaration now uses ES module default export.
This change from export = Push to export default Push correctly aligns with the new ES module build. However, this is a breaking change for TypeScript users who currently use import Push = require('ably/push') syntax—they will need to update to import Push from 'ably/push'.
Verify that this breaking change is documented in the changelog or release notes:
🏁 Script executed:
#!/bin/bash
# Check for changelog or migration documentation
fd -t f -e md . | xargs rg -l "breaking.*change|migration|upgrade" | head -5Length of output: 124
🏁 Script executed:
#!/bin/bash
# Check CHANGELOG.md for mentions of the Push export change
echo "=== Checking CHANGELOG.md for Push/TypeScript export mentions ==="
rg -n "push\.d\.ts|export.*Push|TypeScript.*export|import.*Push" CHANGELOG.md -A2 -B2
echo ""
echo "=== Checking v2 migration guide for TypeScript/Push import changes ==="
rg -n "push\.d\.ts|export.*Push|import.*Push|TypeScript" docs/migration-guides/v2/lib.md -A2 -B2Length of output: 1301
🏁 Script executed:
#!/bin/bash
# Broader search for ESM/default export/Push mentions
echo "=== Checking CHANGELOG.md for default export, ESM, or Push references ==="
rg -n "default export|ESM|^## Push|^### Push" CHANGELOG.md -A3
echo ""
echo "=== Broader search in CHANGELOG for any Push mentions ==="
rg -in "push" CHANGELOG.md | head -20
echo ""
echo "=== Check if push.d.ts is mentioned anywhere in docs ==="
fd -t f \( -name "*.md" -o -name "CHANGELOG*" \) | xargs rg -l "push\.d\.ts"Length of output: 2516
🏁 Script executed:
#!/bin/bash
# Verify the actual push.d.ts file has the "export default Push" syntax
echo "=== Current push.d.ts content (last 10 lines) ==="
tail -10 push.d.ts 2>/dev/null || find . -name "push.d.ts" -type f -exec tail -10 {} \;
echo ""
echo "=== Check git history to confirm this is a change ==="
git log --oneline -n 20 -- "**/push.d.ts" 2>/dev/null | head -10Length of output: 416
Document the TypeScript declaration breaking change in CHANGELOG and migration guide.
The export default Push change from export = is confirmed in the code and documented examples already reflect the new syntax. However, this breaking change is not mentioned in CHANGELOG.md or docs/migration-guides/v2/lib.md, leaving TypeScript users unaware they must update from import Push = require('ably/push') to import Push from 'ably/push'.
Add entries to:
- CHANGELOG.md: Document the push.d.ts export change as a TypeScript breaking change
- docs/migration-guides/v2/lib.md: Include guidance on updating push imports in the "Only TypeScript users" section
🤖 Prompt for AI Agents
In push.d.ts around line 28 the export was changed from CommonJS-style `export =
Push` to ESModule `export default Push`, which is a TypeScript-breaking change;
update CHANGELOG.md to add an entry under the appropriate release noting this
breaking change (mention file push.d.ts and that TypeScript imports must change
from `import Push = require('ably/push')` to `import Push from 'ably/push'`),
and update docs/migration-guides/v2/lib.md in the "Only TypeScript users"
section to include a short migration note and example showing the old import and
the new import syntax plus a one-line explanation to update consumer code.
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
Chores