posthog-js-lite icon indicating copy to clipboard operation
posthog-js-lite copied to clipboard
verified publisher icon PostHog

feat!(node): support sentry>=8

Open songkeys opened this issue 1 year ago • 2 comments

Problem

see https://github.com/getsentry/sentry-javascript/pull/11238

Changes

Change the inner implmenetation of sentry integration.

Release info Sub-libraries affected

Bump level

  • [x] Major
  • [ ] Minor
  • [ ] Patch

Libraries affected

  • [ ] All of them
  • [ ] posthog-web
  • [x] posthog-node
  • [ ] posthog-react-native

Changelog notes

  • Added support for sentry >= 8. No longer supports sentry < 8 as the function signature is changed.

songkeys avatar Jul 12 '24 07:07 songkeys

Any updates on this PR? Looking forward to have it deployed 🙂

alyavasilyeva avatar Jul 24 '24 14:07 alyavasilyeva

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.

songkeys avatar Jul 24 '24 14:07 songkeys

cc @neilkakkar @daibhin in case you wanna test this out before merging. it will likely break people using sentry <= v7 so ideally a changelog + docs would be ideal

marandaneto avatar Oct 15 '24 08:10 marandaneto

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.

songkeys avatar Oct 15 '24 08:10 songkeys

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. If it's possible to detect at runtime and use the right one, we would keep back compatibility and not break people's apps.

marandaneto avatar Oct 15 '24 09:10 marandaneto

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

pauldambra avatar Oct 15 '24 09:10 pauldambra

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.

songkeys avatar Oct 15 '24 09:10 songkeys

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.

songkeys avatar Oct 17 '24 13:10 songkeys

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 https://github.com/PostHog/posthog-js-lite/pull/311 to better align with what we have in posthog-js (I wanted to avoid making the types changes). I'm going to work on shipping that PR tomorrow which should fix your issues

daibhin avatar Nov 20 '24 20:11 daibhin

Closing this as https://github.com/PostHog/posthog-js-lite/pull/311 is now merged! Thank you again @songkeys for taking this on and providing the original direction

daibhin avatar Nov 25 '24 17:11 daibhin