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

ref(v8): change integration.setupOnce signature #11238

Merged
merged 2 commits into from
Mar 22, 2024
Merged

Conversation

AbhiPrasad
Copy link
Member

Make integration.setupOnce accept no arguments. This will allow us to easily remove addGlobalEventProcessor which is deprecated API.

This also means we can remove IntegrationFnResult, as the type signature of the functional and class based integrations are now the same.

Next up - remove addGlobalEventProcessor!

Make integration.setupOnce accept no arguments. This will allow us to
easily remove addGlobalEventProcessor.
@AbhiPrasad AbhiPrasad requested review from a team, mydea and Lms24 and removed request for a team March 22, 2024 02:49
@AbhiPrasad AbhiPrasad self-assigned this Mar 22, 2024
Copy link
Contributor

github-actions bot commented Mar 22, 2024

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) 80.86 KB (added)
@sentry/browser (incl. Tracing, Replay) 72.21 KB (added)
@sentry/browser (incl. Tracing, Replay with Canvas) 76 KB (added)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 65.76 KB (added)
@sentry/browser (incl. Tracing) 36.8 KB (added)
@sentry/browser (incl. browserTracingIntegration) 36.8 KB (added)
@sentry/browser (incl. feedbackIntegration) 31.32 KB (added)
@sentry/browser (incl. feedbackModalIntegration) 31.43 KB (added)
@sentry/browser (incl. feedbackScreenshotIntegration) 31.44 KB (added)
@sentry/browser (incl. sendFeedback) 27.41 KB (added)
@sentry/browser 22.57 KB (added)
CDN Bundle (incl. Tracing, Replay, Feedback) 75.2 KB (added)
CDN Bundle (incl. Tracing, Replay) 70.04 KB (added)
CDN Bundle (incl. Tracing) 36.4 KB (added)
CDN Bundle 23.96 KB (added)
CDN Bundle (incl. Tracing, Replay) - uncompressed 220.02 KB (added)
CDN Bundle (incl. Tracing) - uncompressed 109.97 KB (added)
CDN Bundle - uncompressed 70.99 KB (added)
@sentry/react (incl. Tracing, Replay) 72.19 KB (added)
@sentry/react 22.6 KB (added)

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@AbhiPrasad AbhiPrasad enabled auto-merge (squash) March 22, 2024 14:34
@AbhiPrasad AbhiPrasad merged commit b7bd95f into develop Mar 22, 2024
92 checks passed
@AbhiPrasad AbhiPrasad deleted the abhi-ref-setuponce branch March 22, 2024 14:36
c298lee pushed a commit that referenced this pull request Mar 22, 2024
Make `integration.setupOnce` accept no arguments. This will allow us to
easily remove `addGlobalEventProcessor` which is deprecated API.

This also means we can remove `IntegrationFnResult`, as the type
signature of the functional and class based integrations are now the
same.

Next up - remove `addGlobalEventProcessor`!
c298lee pushed a commit that referenced this pull request Mar 23, 2024
Make `integration.setupOnce` accept no arguments. This will allow us to
easily remove `addGlobalEventProcessor` which is deprecated API.

This also means we can remove `IntegrationFnResult`, as the type
signature of the functional and class based integrations are now the
same.

Next up - remove `addGlobalEventProcessor`!
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this pull request Apr 19, 2024
Make `integration.setupOnce` accept no arguments. This will allow us to
easily remove `addGlobalEventProcessor` which is deprecated API.

This also means we can remove `IntegrationFnResult`, as the type
signature of the functional and class based integrations are now the
same.

Next up - remove `addGlobalEventProcessor`!
@mozeryansky
Copy link

This caused a regression (PostHog/posthog-js#1205) is there a migration guide?

@mydea
Copy link
Member

mydea commented May 27, 2024

Instead of passing these in, you should just use the global methods Sentry.xxx directly in your integration. So e.g. migrating this:

const integration = {
  setupOnce(addGlobalEventProcessor, getCurrentHub) {
    // do something...
  }
}

Do this:

const integration = {
  setupOnce() {
    Sentry.addEventProcessor(...);
    // use respective replacement of hub
  }
}

We do not have good docs on this yet, we'll update them accordingly!

In most places, you should not use setupOnce anymore at all - only use this when e.g. wrapping APIs or similar. Instead, use these new hooks:

const integration = {
  processEvent(event, hint) {
     // use this directly if you just want to process events
    return event;
  },
  setup(client) {
    // use this for things you want to run once on setup of your app/client
  }
}

@mozeryansky
Copy link

Thanks! I was able to get it working here. It's a 3rd party library so they'll need to update it.

@mydea
Copy link
Member

mydea commented May 27, 2024

For reference, and people stumbling over this later, here the PR to update the docs for the integration interface: getsentry/sentry-docs#10150

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

Successfully merging this pull request may close these issues.

4 participants