Uh oh!
There was an error while loading. Please reload this page.
fix(esbuild): Avoid warning when entry point has no default export - #901
fix(esbuild): Avoid warning when entry point has no default export#901queelius wants to merge 4 commits into
Conversation
When using @sentry/esbuild-plugin with an entry point that has no default export (common for Express.js apps), the proxy module generates `export default OriginalModule.default` which triggers esbuild's "import-is-undefined" warning. Fix by wrapping the default export access in a function call, which prevents esbuild's static analysis from detecting the potentially missing default while still correctly forwarding it when present. Fixesgetsentry#747 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an esbuild (0.25+) warning emitted by the esbuild debug-ID proxy module when the wrapped entry point has no default export (common for Express-style “no exports” entrypoints).
Changes:
- Update the generated proxy module to access the default export via a function wrapper to avoid triggering esbuild’s
import-is-undefinedstatic analysis warning. - Add unit tests covering entry points with no default export, with a default export, and with named-only exports.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/esbuild-plugin/src/index.ts | Adjusts the proxy module code generation to avoid default export warning while preserving behavior. |
packages/esbuild-plugin/test/no-default-export-warning.test.ts | Adds tests ensuring the build produces no import-is-undefined warnings and preserves exports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let tmpDir: string; | ||
| beforeAll(() => { | ||
| tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sentry-esbuild-test-")); | ||
| }); | ||
| afterAll(() => { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
afterAll unconditionally calls fs.rmSync(tmpDir, ...) but tmpDir is only set in beforeAll. If beforeAll fails (or a test is skipped before initialization), the cleanup can throw and mask the original failure. Consider guarding the cleanup (e.g., only remove when tmpDir is set/existing).
| lettmpDir: string; | |
| beforeAll(()=>{ | |
| tmpDir=fs.mkdtempSync(path.join(os.tmpdir(),"sentry-esbuild-test-")); | |
| }); | |
| afterAll(()=>{ | |
| fs.rmSync(tmpDir,{recursive: true,force: true}); | |
| lettmpDir: string|undefined; | |
| beforeAll(()=>{ | |
| tmpDir=fs.mkdtempSync(path.join(os.tmpdir(),"sentry-esbuild-test-")); | |
| }); | |
| afterAll(()=>{ | |
| if(tmpDir){ | |
| fs.rmSync(tmpDir,{recursive: true,force: true}); | |
| } |
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Apply oxfmt formatting rules to satisfy CI formatting check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
timfish
commented
May 4, 2026
I have concerns about the side-effects this adds to every bundle |
Summary
Fixes#747
When using
@sentry/esbuild-pluginwith an entry point that has no default export (common for Express.js apps), the plugin's debug ID proxy module generates:This causes esbuild (0.25+) to emit:
The fix wraps the default export access in a function call, which prevents esbuild's static analysis from detecting the potentially missing
.defaultproperty while still correctly forwarding it when present:Test plan
vitest runinpackages/esbuild-plugin)yarn testinpackages/integration-tests)eslint ./src ./test)🤖 Generated with Claude Code