Skip to content
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

Lots of globals detected when running with mocha --check-leaks #2215

Closed
4 of 8 tasks
ricardograca opened this issue Aug 27, 2019 · 6 comments
Closed
4 of 8 tasks

Lots of globals detected when running with mocha --check-leaks #2215

ricardograca opened this issue Aug 27, 2019 · 6 comments

Comments

@ricardograca
Copy link

Package + Version

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

Version:

5.6.2

Description

I just updated the Sentry client from version 4.0.5 to 5.6.2 and now my test suite is complaining about a ton of global leaks. This seems to be caused by the tslib package: microsoft/tslib#32.

I can accept __SENTRY__ as a requirement, but these other globals don't seem to be necessary for Sentry to work properly, so they should not be exposed like that.

Here is the error for reference:

Error: global leaks detected: '__extends', '__assign', '__rest', '__decorate', '__param', '__metadata', '__awaiter', '__generator', '__exportStar', '__values', '__read', '__spread', '__await', '__asyncGenerator', '__asyncDelegator', '__asyncValues', '__makeTemplateObject', '__importStar', '__importDefault'
@kamilogorek
Copy link
Contributor

@ricardograca are you using CDN or npm package? I just verified CDN one, and it doesn't do that:

image

@ricardograca
Copy link
Author

I thought that by ticking @sentry/node above it was implied that I was using npm package. This is in a Node.js project.

@kamilogorek
Copy link
Contributor

Yes, I just missed that. And I'm not sure if/how we can do anything about it other than escalating higher to the tslib itself.

@ricardograca
Copy link
Author

I've seen a comment there in the linked issue that mentions a possible workaround, but I don't know if it's feasible: microsoft/tslib#32 (comment)

@kamilogorek
Copy link
Contributor

Yeah I've seen this code, but it's not applicable to node apps. If we do this, we'd, have to maintain our own vendor file of tslib and ship it with the package itself. And this, on the other hand, would prevent npm/yarn to be able to dedupe it if any other package has it as a dependency, which would cause duplication and downloading of the same code twice.

@kamilogorek
Copy link
Contributor

Closing the issue as a part of large repository cleanup, due to it being inactive and/or outdated.
Please do not hesitate to ping me if it is still relevant, and I will happily reopen and work on it.
Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants