fix: Improve ESM compatibility for named exports#2724
fix: Improve ESM compatibility for named exports#2724WilliamBergamin merged 6 commits intoslackapi:mainfrom
Conversation
This commit fixes ESM interoperability issues identified by @arethetypeswrong/cli
where named exports weren't accessible when importing from ESM.
Changes:
- Refactor receiver exports to use import-then-export pattern instead of
`export { default as X }` syntax
- Separate type-only exports using `export type` for better tree-shaking
- Fix Logger and OAuth type re-exports to be type-only
The previous pattern `export { default as ExpressReceiver } from './receivers/ExpressReceiver'`
generated code with `__importDefault().default` which isn't statically analyzable
by Node.js's ESM-CJS interop layer.
The new pattern imports first then re-exports:
```typescript
import ExpressReceiver from './receivers/ExpressReceiver';
export { ExpressReceiver };
export type { ExpressReceiverOptions } from './receivers/ExpressReceiver';
```
This generates simpler CommonJS code that Node.js can properly expose as named
exports when imported from ESM.
Verified with @arethetypeswrong/cli - NamedExports errors are now resolved.
Relates to #2565, #2632, #2644
|
Thanks for the contribution! Before we can merge this, we need @grantjoy to sign the Salesforce Inc. Contributor License Agreement. |
|
Hi @grantjoy 👋🏻 Thanks for taking the time to craft an in-depth and quality pull request! 👌🏻 The PR looks safe to run our GitHub Workflows, so I've approved it. It looks like there are some minor failures due to biomjs formatting: ❓ When you have a moment, would you mind pushing a commit to fix the formatting error? It looks like the newline on L95 should be removed. 🔍 I'll ping a few teammates to look over your pull request, but on first review it's look good to move forward! 👍🏻 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2724 +/- ##
=======================================
Coverage 93.44% 93.44%
=======================================
Files 37 37
Lines 7675 7675
Branches 669 669
=======================================
Hits 7172 7172
Misses 498 498
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
This PR fixes ESM interoperability issues where named exports (particularly
ExpressReceiver,SocketModeReceiver,HTTPReceiver, andAwsLambdaReceiver) weren't accessible when importing from ESM, as identified by@arethetypeswrong/cli.Problem
The current export pattern:
Generates CommonJS code like:
This dynamic property access pattern isn't statically analyzable by Node.js's ESM-CJS interop layer, so these exports don't get exposed when importing from ESM:
Solution
Refactored to use an import-then-export pattern:
This generates simpler, statically analyzable CommonJS code that Node.js can properly expose as named exports in ESM.
Additionally:
export typefor better tree-shakingLoggerand OAuth type re-exports to be type-only (they're interfaces, not runtime values)Verification
Before:
{ "problems": { "NamedExports": [ { "missing": [ "Logger", "ExpressReceiver", "SocketModeReceiver", "HTTPReceiver", "AwsLambdaReceiver", "Installation", "InstallURLOptions", "InstallationQuery", "InstallationStore", "StateStore", "InstallProviderOptions" ] } ] } }After:
$ bunx @arethetypeswrong/cli --pack . -f json{ "problems": {} }✅ All
NamedExportsissues resolved!ESM Import Test:
CommonJS Compatibility:
Tests
All existing tests pass:
Related Issues
Migration Guide
This change is fully backward compatible. No code changes required for consumers.
Both import styles continue to work: