search

PostHog / posthog-js-lite

Reimplementation of posthog-js to be as light and modular as possible.
https://posthog.com/docs/libraries
MIT License
69 stars 36 forks source link

feat!(node): support sentry>=8 #243

Open songkeys opened 4 months ago

songkeys commented 4 months ago

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

Libraries affected

Changelog notes

alyavasilyeva commented 4 months ago

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

songkeys commented 4 months ago

I have currently released it within my scope (@songkeys/posthog-node@5.0.0-beta.0). 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.

marandaneto commented 1 month ago

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

songkeys commented 1 month ago

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.

marandaneto commented 1 month ago

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.

pauldambra commented 1 month ago

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

songkeys commented 1 month ago

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 commented 1 month ago

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.

daibhin commented 4 days ago

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