TypeScript w/ esModuleInterop: import Sentry from "@sentry/{node|browser}" is not an error, but does not work.

[krisdages]

• Review the documentation: https://docs.sentry.io/
• Search for existing issues: https://github.com/getsentry/sentry-javascript/issues
• Use the latest release: https://github.com/getsentry/sentry-javascript/releases
• N/A - Provide a link to the affected event from your Sentry account

Relates to PR #3077

Package + Version

• @sentry/browser
• @sentry/node
• raven-js
• raven-node (raven for node)
• other:

Version:

5.29.0

Description

Sentry must be imported using import * as Sentry instead of import Sentry in order to work.
With the esModuleInterop compiler option enabled, TypeScript does not complain about import Sentry.
(With the option off, TS recognizes that the module does not have a default import and forbids import Sentry)

This appears to be because the Sentry index.js module declares __esModule: true but does not actually have a value for the default export:

 // import Sentry from "@sentry/node" - transpiled
 node_1 = tslib_1.__importDefault(node_1);

 //tslib
 __importDefault = function (mod) {
        return (mod && mod.__esModule) ? mod : { "default": mod };
    };

Can the library be updated so the default import works as TypeScript thinks it does?
This is only an issue when the esModuleInterop setting is on, but it's a pretty valuable setting and a dangerous mistake
for the developer.

// Either 
// vv - Can this be added?
exports.default = exports;

// Or
// vv - Can this be removed?  Though it would make the `import *` less efficient. 
Object.defineProperty(exports, "__esModule", { value: true });

Thanks!

[bodinsamuel]

+1 on this
The typescript definition does not report an error but it's indeed broken

[killthekitten]

Same goes for @sentry/serverless. Search terms:

Cannot read property 'GCPFunction' of undefined

[fosrias]

@rbisol Why is this not a bug. It basically makes this library unusable when you are working in an environment requiring interop?

[lo1tuma]

I’ve ran into this problem as well. Since we are using dependency injection we almost shipped a broken version of our app to production.

[florian-milky]

FWIW, if I remove this line from the commonJS module:
Object.defineProperty(exports, "__esModule", { value: true });, it works as expected

[imsamurai]

any news?

[lforst]

@imsamurai is this still an issue with the newest version of the SDK? We updated how we bundle the SDK in version 7.x.x.

[incompletude]

still a problem, no default export

[amc6]

This is still a problem. Is there any fix in the works? This is incredibly dangerous especially for a module that tends to be used as part of error handling. Its very easy for this to go unnoticed until a bug causes a new exception. As an example, Sentry.captureException will generally only be called in "exceptional" circumstances and you may not have comprehensive tests for every place you call Sentry.captureException. But its a big problem for that call to fail as it breaks error reporting and causes an entirely different error in error handling code

[mydea]

HI @amc6,

could you share more details in how you ran into this? We do not document import Sentry from '@sentry/xxx' anywhere (because it does not work), and our TS setup should be setup properly - it is not incorrect to have no default export in ESM, as far as I can tell.

So could you share:

• your tsconfig
• The code that uses/references Sentry
• Why/how you came to use a default import

[lforst]

@mydea I did some digging around this issue a while ago and I think we have to play around with enableLegacyTypeScriptModuleInterop.

[getsantry]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[Naddiseo]

This is still an issue, the issue can stay open

[getsantry]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[lforst]

Even though we are not working on this right now, we still shouldn't close it. The issue has 20 thumbs.

[lforst]

Okay, I have been looking at this again. I hope I won't make myself too unpopular with the following but I believe we should close this issue without particularly fixing it.

The way we currently build our packages

• we basically never have any default exports and don't plan on having any.
• we add a Object.defineProperty(exports, "__esModule", { value: true }); to all CJS files.

As I understand it, the Object.defineProperty(exports, "__esModule", { value: true }); is not technically wrong, it may just trick certain tools into thinking that there is a default export. I am inclined to change this by setting the rollup output.esModule option to "if-default-prop". @florian-milky mentioned that they removed it from the CJS module, and that it fixed the issue - which would be great. However, from my testing, TypeScript unfortunately still didn't complain as long as I had the esModuleInterop option enabled.

I also looked into Rollup's output.interop option but I don't think it will help us because it only is about importing not exporting.

So effectively we are making two calls:

• We won't add any default exports.
  • We don't need them.
  • They would add a bit of bundle size.
  • I believe they inhibit tree-shaking, effectively handing people a footgun.
• We will remove the __esModule thing, except for in modules where we actually have default exports. It seems, more correct that way but I don't think it will fix this issue.

If anybody else has input or suggestions on how to resolve the topic please reach out. I may be missing something.

[jfirebaugh]

https://arethetypeswrong.github.io/?p=%40sentry%2Fbrowser%408.48.0 correctly diagnoses the issue here: when imported from ESM, @sentry/browser is masquerading as CJS, because it attempts to use the same type definition file for both CJS and ESM implementation files. A single type definition file cannot represent JavaScript files of two different module formats. As described in the above link, a typical solution is to provide a separate .d.mts definition file for the ESM implementation file.

[lforst]

@jfirebaugh dang this looks promising. Thank you so much! I'll investigate further.