Skip to content

Conversation

lforst
Copy link
Contributor

@lforst lforst commented Nov 28, 2024

Done as part of #14265

We are exposing the vueIntegration so we can streamline how component tracking is configured.

Copy link
Contributor

github-actions bot commented Nov 29, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.1 KB - -
@sentry/browser - with treeshaking flags 21.83 KB - -
@sentry/browser (incl. Tracing) 35.5 KB - -
@sentry/browser (incl. Tracing, Replay) 72.39 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.87 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 76.71 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 89.16 KB - -
@sentry/browser (incl. Feedback) 39.84 KB - -
@sentry/browser (incl. sendFeedback) 27.72 KB - -
@sentry/browser (incl. FeedbackAsync) 32.53 KB - -
@sentry/react 25.8 KB - -
@sentry/react (incl. Tracing) 38.4 KB - -
@sentry/vue 27.25 KB - -
@sentry/vue (incl. Tracing) 37.3 KB - -
@sentry/svelte 23.25 KB - -
CDN Bundle 24.32 KB - -
CDN Bundle (incl. Tracing) 37.2 KB - -
CDN Bundle (incl. Tracing, Replay) 72.08 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 77.41 KB - -
CDN Bundle - uncompressed 71.45 KB - -
CDN Bundle (incl. Tracing) - uncompressed 110.5 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 223.57 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 236.79 KB - -
@sentry/nextjs (client) 38.71 KB - -
@sentry/sveltekit (client) 36.05 KB - -
@sentry/node 135.08 KB - -
@sentry/node - without tracing 97.13 KB - -
@sentry/aws-serverless 109.43 KB - -

View base workflow run

@lforst lforst requested a review from s1gr1d November 29, 2024 09:10
@lforst lforst marked this pull request as ready for review November 29, 2024 09:10
@lforst lforst requested a review from mydea November 29, 2024 09:29
Comment on lines +58 to +64
vueIntegration({
// We pick up the options from the "fake" vueIntegration exported by the SDK.
...(GLOBAL_OBJ as GlobalObjWithIntegrationOptions)._sentryNuxtVueIntegrationOptions,
app: vueApp,
attachErrorHandler: false,
}),
);
Copy link
Member

Choose a reason for hiding this comment

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

Just a question: If users add a vueIntegration as well, can this even be another another time? I remember it's not possible to add the same integration twice.

What we could do is removing this addIntegration part here and add the vueIntegration as default integration in the default integration array (users can overwrite those integrations). As we need to get the vueApp in the integration, we can get it from window.useNuxtApp().vueApp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why I immediately went with this solution is because I thought the vueApp is initialized rather late - at least later than Sentry.init() is called. In that case, we would initialize the SDK (and call the integrations' setup method) before window.useNuxtApp() is even available. Is my concern valid? I think you know more than me in this regard.

If users add a vueIntegration as well

I don't think I fully understand this question. Can you clarify a bit?

How this PR is currently implemented, the configuration added by the user would "win" and their options are used when we add the vueIntegration() in the nuxt plugin setup hook.

The entire point of this PR is that users can add their own vueIntegration(). Maybe I am also misunderstanding. In case you are worried about "us adding the integration twice", that is actually not the case in this PR - the name between the @sentry/nuxt vueIntegration() and the @sentry/vue integration differs slightly, so they will both be set up.

Copy link
Member

Choose a reason for hiding this comment

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

Is my concern valid?

In the client-case, the Nuxt app is initialized before Sentry. I haven't tried so far but it should work in theory.

integration differs slightly

True, I wasn't thinking about that...those are two different integrations and not the same. And the new Nuxt vueIntegration is only setting the options, the logic is inside Vue vueIntegration which will still be added but with the added options? I understood it now :D
All good - I should have read the comment in the integration

@lforst lforst merged commit 9b9ec77 into develop Dec 2, 2024
153 checks passed
@lforst lforst deleted the lforst-expose-vue-integration-in-nuxt branch December 2, 2024 08:49
jimmcq added a commit to jimmcq/mars-weather-dashboard that referenced this pull request Sep 29, 2025
Resolves critical build failure caused by outdated Sentry configuration
incompatible with Next.js 15 App Router.

Changes:
- Created src/instrumentation.ts for server-side Sentry initialization
- Created src/instrumentation-client.ts for client-side initialization
- Added src/app/global-error.tsx to handle React rendering errors
- Added src/app/not-found.tsx for custom 404 page
- Removed deprecated sentry.*.config.ts files
- Updated Sentry packages to v10.16.0
- Removed old Sentry import from layout.tsx

Technical Details:
- Sentry's withSentryConfig wrapper temporarily disabled due to Next.js 15
  compatibility issue (generates Pages Router error pages incompatible with
  App Router)
- Implemented manual instrumentation hooks following Next.js App Router
  patterns
- Added onRequestError and onRouterTransitionStart exports
- Build now succeeds with proper NODE_ENV=production

Testing:
- ✅ Production build passes (yarn build:ci)
- ✅ All 254 tests passing
- ✅ TypeScript compilation successful
- ⚠️  ESLint warning in useMartianTime.ts (pre-existing, tracked separately)

References:
- https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/
- getsentry/sentry-javascript#14526
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.

2 participants