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

deps(raven): make raven an optional peer dependency #13020

Closed
wants to merge 1 commit into from
Closed

deps(raven): make raven an optional peer dependency #13020

wants to merge 1 commit into from

Conversation

remcohaszing
Copy link

Summary

This package has been deprecated in favor of @sentry/node. It just hasn’t been marked as such on npmjs.com.

It depends on deprecated packages, which yields deprecation warnings for users installing lighthouse.

It’s usage is optional in lighthouse. It’s disabled by default, and even if it’s enabled in options, the require statement for it is in a try-catch block.

Related Issues/PRs

https://docs.sentry.io/clients/javascript/

@remcohaszing remcohaszing requested a review from a team as a code owner September 8, 2021 09:25
@remcohaszing remcohaszing requested review from adamraine and removed request for a team September 8, 2021 09:25
@google-cla google-cla bot added the cla: yes label Sep 8, 2021
@remcohaszing remcohaszing changed the title Make raven an optional peer dependency deps(raven) make raven an optional peer dependency Sep 15, 2021
@remcohaszing remcohaszing changed the title deps(raven) make raven an optional peer dependency deps(raven): make raven an optional peer dependency Sep 15, 2021
This package has been deprecated in favor of `@sentry/node`. It just hasn’t been
marked as such on npmjs.com.

It depends on deprecated packages, which yields deprecation warnings for users
installing lighthouse.

It’s usage is optional in lighthouse. It’s disabled by default, and even if it’s
enabled in options, the `require` statement for it is in a try-catch block.
@connorjclark
Copy link
Collaborator

connorjclark commented Jan 31, 2022

As this is our error reporting service, we can't expect users to manually install it for us to continue to receive error reports, so we can't accept this PR. Are you aware of other projects installing this package on an as-needed basis?

We do have this PR I started but abandoned due to prioritization: #9325

@remcohaszing
Copy link
Author

I was under the impression error reporting was disabled by default, but I was wrong. The user is prompted for explicit consent.

if (typeof cliFlags.enableErrorReporting === 'undefined') {
cliFlags.enableErrorReporting = await askPermission();
}

Personally I was a bit triggered by the deprecation messages displayed when running npm install lighthouse. uuid@3 is one of those, which is installed as a dependency of raven.

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

Successfully merging this pull request may close these issues.

4 participants