feat(nuxt): Add vue-router instrumentation#13054
Conversation
There was a problem hiding this comment.
This is not related - just a restructuring of the type
| export default defineNuxtPlugin(nuxtApp => { | ||
| nuxtApp.hook('app:created', vueApp => { | ||
| const sentryClient = getClient(); | ||
| const sentryClient = getClient(); |
There was a problem hiding this comment.
l: We can early return if client is not defined to remove the if (sentryClient) checks below.
There was a problem hiding this comment.
I'd rather leave the checks as I'm also accessing the client in a Nuxt lifecycle hook and I cannot be sure that the early return would hinder the lifecycle hook to be called if there is no client. Does this make sense?
There was a problem hiding this comment.
I see what you mean. I think then we need a new call to getClient inside the hook so we ensure that we always have an up-to-date reference.
packages/nuxt/src/common/types.ts
Outdated
| type SourceMapsOptions = { | ||
| /** | ||
| * Options for the Sentry Vite plugin to customize the source maps upload process. | ||
| * If this flag is `true`, and an auth token is detected, the Sentry integration will |
There was a problem hiding this comment.
l (sorry for the bike shedding): Just was wondering if "integration" is a good term here? IIUC users interact with our nuxt module here, correct? So I'd go with module or just SDK.
| * If this flag is `true`, and an auth token is detected, the Sentry integration will | |
| * If this flag is `true`, and an auth token is detected, the Sentry integration will |
| if (sentryClient && '$router' in nuxtApp) { | ||
| sentryClient.addIntegration( | ||
| browserTracingIntegration({ router: nuxtApp.$router as VueRouter, routeLabel: 'path' }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
m: This is runtime code, right? If so, I think we need to add the __SENTRY_TRACING__ guard here to enable tree shaking of browserTracingIntegration.
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Thanks for addressing my review! LGTM!
3 participants
Adding the Vue BrowserTracing instrumentation to get parametrized routes.
closes #13067