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

krisdages commented 3 years ago

[X] Review the documentation: https://docs.sentry.io/
[X] Search for existing issues: https://github.com/getsentry/sentry-javascript/issues
[X] 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 https://github.com/getsentry/sentry-javascript/pull/3077

Package + Version

[X] @sentry/browser
[X] @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 commented 3 years ago

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

killthekitten commented 3 years ago

Same goes for @sentry/serverless. Search terms:

Cannot read property 'GCPFunction' of undefined

fosrias commented 3 years ago

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

lo1tuma commented 3 years ago

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 commented 3 years ago

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

imsamurai commented 2 years ago

any news?

lforst commented 2 years ago

@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 commented 1 year ago

still a problem, no default export

amc6 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

getsantry[bot] commented 5 months ago

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 commented 5 months ago

This is still an issue, the issue can stay open

getsantry[bot] commented 3 months ago

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 commented 3 months ago

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

getsentry / sentry-javascript