-
Notifications
You must be signed in to change notification settings - Fork 255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
node: TypeScript and rollup #2286
Conversation
__BUGSNAG_NOTIFIER_VERSION__: JSON.stringify(packageJson.version), | ||
__BUGSNAG_NOTIFIER_VERSION__: packageJson.version, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reverted how the version handling is done across all packages because I'd misunderstood how it was working. As can be seen below the version is injected inside a string rather than as a variable. This is how it was done before. The problem with it being a variable is that outside of rollup you can't import or require the file without somehow defining the global variable first. This is done in some of the integration tests for example.
{ | ||
"extends": "../../tsconfig.json", | ||
"compilerOptions": { | ||
"types": ["jest", "node"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned before, the root tsconfig.json
has "types: ["jest"]
, which excludes other types - but we need @types/node
here hence why it is redefined.
It would be better to remove "types: ["jest"]
from the root tsconfig.json
but doing so means that @types/react-native
are also included, which then conflict with lib dom, as defined by "lib": [ "dom", "esnext" ],
.
So to do that we'll need to restructure the way jest / typescript is set up. One for a later PR.
Goal
Convert
@bugsnag/node
to TypeScript and bundle with rollup, replacing the browserify bundling.Design
Consistency with new approach to other packages. Needed doing before the
@bugsnag/core
conversion becausebrowserify
does not understandpackage.exports
(see #2296).Testing
e2e tests passing