Skip to content

feat(core): Add client.getIntegrationByName()#10130

Merged
mydea merged 1 commit intodevelopfrom
fn/getIntegrationByName
Jan 15, 2024
Merged

feat(core): Add client.getIntegrationByName()#10130
mydea merged 1 commit intodevelopfrom
fn/getIntegrationByName

Conversation

@mydea
Copy link
Copy Markdown
Member

@mydea mydea commented Jan 10, 2024

And also deprecate both getIntegration() as well as getIntegrationById() which is only on the baseclient, but not on the client type, anyhow :grimace:.

Usage:

const replay = getClient().getIntegrationByName('Replay');

Or, if you want to have an easier time with types:

const replay = getClient().getIntegrationByName<Replay>('Replay');

Why do we need this

In v8, integrations will not be classes anymore, so you cannot pass a class definition anymore to getIntegration(). We also decided to remove the static id field, so you cannot find an integration via that way anymore.

getIntegrationById() was always just on the baseclient, so not properly types anyhow. It is also an inconsistent/weird naming because we will not have an id anymore for integrations at all.

@mydea mydea requested review from AbhiPrasad, Lms24 and lforst January 10, 2024 11:01
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 10, 2024

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.01 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 68.38 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.04 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 32.41 KB (+0.06% 🔺)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31 KB (+0.03% 🔺)
@sentry/browser - Webpack (gzipped) 22.34 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 74.7 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 66.35 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 32.18 KB (+0.09% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 23.97 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 209 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 97.05 KB (+0.08% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 71.65 KB (+0.09% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 35.18 KB (+0.05% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 68.76 KB (+0.03% 🔺)
@sentry/react - Webpack (gzipped) 22.38 KB (+0.05% 🔺)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 85.42 KB (+0.03% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 49.53 KB (+0.05% 🔺)
@sentry-internal/feedback - Webpack (gzipped) 17 KB (0%)

@mydea mydea force-pushed the fn/getIntegrationByName branch from bad4f1e to da90b3c Compare January 10, 2024 12:38
Copy link
Copy Markdown
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! I think this is way cleaner naming-wise and passing a type as a generic makes sense to me!

@mydea mydea force-pushed the fn/getIntegrationByName branch 2 times, most recently from 0421747 to 1ad9c60 Compare January 15, 2024 08:17
And also deprecate both `getIntegration()` as well as `getIntegrationById()` which is only on the baseclient, but not on the client type, anyhow :grimace:.

Usage:

```ts
const replay = getClient().getIntegrationByName('Replay');
```

Or, if you want to have an easier time with types:

```ts
const replay = getClient().getIntegrationByName<Replay>('Replay');
```
@mydea mydea force-pushed the fn/getIntegrationByName branch from 1ad9c60 to b7b094e Compare January 15, 2024 10:30
@mydea mydea merged commit 5498a78 into develop Jan 15, 2024
@mydea mydea deleted the fn/getIntegrationByName branch January 15, 2024 12:27
@pckilgore
Copy link
Copy Markdown

pckilgore commented Feb 6, 2024

Would appreciate a helper type for the new value integrations this is a bit wild right now:

  const replayId = Sentry.getClient()
    ?.getIntegrationByName?.<ReturnType<typeof Sentry.replayIntegration>>(
      'Replay'
    )
    ?.getReplayId();

FWIW, I see it still resolves to the class for now, so this is much ado about linter warnings.

Would also appreciate it if name could be an enum discriminant against a union to avoid the generic / me having to find this PR to figure out what a valid value might be 😆.

@mydea
Copy link
Copy Markdown
Member Author

mydea commented Feb 6, 2024

Would appreciate a helper type for the new value integrations this is a bit wild right now:

  const replayId = Sentry.getClient()
    ?.getIntegrationByName?.<ReturnType<typeof Sentry.replayIntegration>>(
      'Replay'
    )
    ?.getReplayId();

FWIW, I see it still resolves to the class for now, so this is much ado about linter warnings.

Would also appreciate it if name could be an enum discriminant against a union to avoid the generic / me having to find this PR to figure out what a valid value might be 😆.

Yeah, I can see that. Maybe we need a helper specifically for replay 🤔 IMHO replay is the only integration where this is really relevant, the idea for integrations overall is that you don't need to access them really ever. I'll see if I can whip up something!

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.

3 participants