-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat!(node): support sentry>=8 #243
Conversation
Any updates on this PR? Looking forward to have it deployed 🙂 |
I have currently released it within my scope (@songkeys/[email protected]). I have been using it in production for several weeks. If you require it urgently, feel free to try it now. (The API is not changed.) Otherwise, please subscribe to this PR and stay updated on the latest developments. |
cc @neilkakkar @daibhin in case you wanna test this out before merging. |
It will certainly cause issues for users on v7. We'll need to bump to a major version, or implement different handling logics depending on the detected version of the sentry peer dependency. |
that would be even cooler, if its possible to keep the old and the new one, like duplicate the code and let people use either, eg sentry-integration-v7 (sentry-integration is an alias to sentry-integration-v7) and sentry-integration-v8. |
yep, we do that in posthog-js (With a very subtle name difference 😅) https://github.com/PostHog/posthog-js/blob/main/src/extensions/sentry-integration.ts#L135-L145 |
Yeah. I think it's feasible. Give me some time to write this up then. Will do this later when I'm off my work. |
Just pushed the change. The current API (with 1 method and 1 class exported just like posthog-js) is not ideal. But it's the only way we can do to get people using sentry@8 onboard while not releasing a major version. Please take a look. |
Thank you so much for taking this on @songkeys, it was a great basis for the changes we needed to make here. I made my own PR in #311 to better align with what we have in |
Problem
see getsentry/sentry-javascript#11238
Changes
Change the inner implmenetation of sentry integration.
Release info Sub-libraries affected
Bump level
Libraries affected
Changelog notes