zivl / sentry-testkit

A Sentry plugin to allow Sentry report interception and further inspection of the data being sent
https://zivl.github.io/sentry-testkit/
MIT License
112 stars 26 forks source link

Fix default export when importing in pure node ESM #145

Closed ohana54 closed 1 year ago

ohana54 commented 2 years ago

Importing sentry-testkit in a pure ESM node project currently fails as the default export doesn't conform to what node expects.

Sample node code:

import sentryTestkit from 'sentry-testkit';

sentryTestkit()

Running the above with an ESM package and node --experimental-vm-modules will result in TypeError: sentryTestkit is not a function

According to node docs, a commonjs default export should be module.exports = X, but for sentry-testkit TS emits exports.default = X. I assume this is related to legacy/browser ESM and backward compatibility, but I don't know enough about this subject. EDIT: Indeed this is related to ES6 modules standard: https://stackoverflow.com/a/50948000

This PR changes the export according to the TS docs.

EDIT: Changed the code to preserve compatibility with const createTestkit = require('sentry-testkit').default According to this article: https://remarkablemark.org/blog/2020/05/05/typescript-export-commonjs-es6-modules/

zivl commented 2 years ago

@ohana54 is this breaking backward compatibility?

ohana54 commented 2 years ago

I'm no expert, but I tested it both on a TS project and plain JS project (CJS and ESM) and it works fine. I first wrote code that uses the latest version, and then replaced it with the locally built testkit - everything worked without a code change. So as far as I could test - no breaking changes.

ohana54 commented 2 years ago

Actually, I now see that types break for a CJS project :( I'm checking if there's a way to fix it.

ohana54 commented 2 years ago

Pushed a fix

PhilippSchreiber commented 2 years ago

Have the problem here. Downgraded to 4.1.0 and everything works fine again.

ohana54 commented 2 years ago

@PhilippSchreiber thanks for the info. This PR is still open as I intend to add tests, and then we'll merge it. Hopefully I'll get to it in the coming month.

ohana54 commented 1 year ago

@zivl added some tests, LMK if that's good enough

zivl commented 1 year ago

@ohana54 thanks!! I'll merge and release next week