-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(nuxt): Setup source maps with vite config #13018
Changes from all commits
b0523a2
2a9e671
ea78994
74ee389
02a2f33
6bc94dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import type { Nuxt } from '@nuxt/schema'; | ||
import { sentryVitePlugin } from '@sentry/vite-plugin'; | ||
import type { SentryNuxtModuleOptions } from '../common/types'; | ||
|
||
/** | ||
* Setup source maps for Sentry inside the Nuxt module during build time. | ||
*/ | ||
export function setupSourceMaps(moduleOptions: SentryNuxtModuleOptions, nuxt: Nuxt): void { | ||
nuxt.hook('vite:extendConfig', async (viteInlineConfig, _env) => { | ||
const sourceMapsUploadOptions = moduleOptions.sourceMapsUploadOptions || {}; | ||
|
||
if ((sourceMapsUploadOptions.enabled ?? true) && viteInlineConfig.mode !== 'development') { | ||
const sentryPlugin = sentryVitePlugin({ | ||
org: sourceMapsUploadOptions.org ?? process.env.SENTRY_ORG, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: Do we need to pass in the env variable fallbacks explicitly? the plugin itself should pick them up, right? |
||
project: sourceMapsUploadOptions.project ?? process.env.SENTRY_PROJECT, | ||
authToken: sourceMapsUploadOptions.authToken ?? process.env.SENTRY_AUTH_TOKEN, | ||
telemetry: sourceMapsUploadOptions.telemetry ?? true, | ||
sourcemaps: { | ||
assets: sourceMapsUploadOptions.sourcemaps?.assets ?? undefined, | ||
ignore: sourceMapsUploadOptions.sourcemaps?.ignore ?? undefined, | ||
filesToDeleteAfterUpload: sourceMapsUploadOptions.sourcemaps?.filesToDeleteAfterUpload ?? undefined, | ||
}, | ||
_metaOptions: { | ||
telemetry: { | ||
metaFramework: 'nuxt', | ||
}, | ||
}, | ||
debug: moduleOptions.debug ?? false, | ||
}); | ||
|
||
viteInlineConfig.plugins = viteInlineConfig.plugins || []; | ||
viteInlineConfig.plugins.push(sentryPlugin); | ||
|
||
const sourceMapsPreviouslyEnabled = viteInlineConfig.build?.sourcemap; | ||
|
||
if (moduleOptions.debug && !sourceMapsPreviouslyEnabled) { | ||
// eslint-disable-next-line no-console | ||
console.log('[Sentry]: Enabled source maps generation in the Vite build options.'); | ||
if (!moduleOptions.sourceMapsUploadOptions?.sourcemaps?.filesToDeleteAfterUpload) { | ||
// eslint-disable-next-line no-console | ||
console.warn( | ||
`[Sentry] We recommend setting the \`sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload\` option to clean up source maps after uploading. | ||
[Sentry] Otherwise, source maps might be deployed to production, depending on your configuration`, | ||
); | ||
} | ||
} | ||
|
||
viteInlineConfig.build = viteInlineConfig.build || {}; | ||
viteInlineConfig.build.sourcemap = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m: If |
||
} | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,8 +108,8 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug | |
|
||
// Modify the config to generate source maps | ||
config: config => { | ||
const sourceMapsPreviouslyEnabled = !config.build?.sourcemap; | ||
if (debug && sourceMapsPreviouslyEnabled) { | ||
const sourceMapsPreviouslyNotEnabled = !config.build?.sourcemap; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thx for fixing! :) |
||
if (debug && sourceMapsPreviouslyNotEnabled) { | ||
// eslint-disable-next-line no-console | ||
console.log('[Source Maps Plugin] Enabeling source map generation'); | ||
if (!mergedOptions.sourcemaps?.filesToDeleteAfterUpload) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lock changes are from this PR: #12920 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, no strong feelings, but it feels a bit confusing that this only turns on debug mode for the vite plugin, right? It seems easy to think that this also enables debug mode for sentry itself 🤔 would it make sense to name this in a different, more specific way, e.g.
viteDebug
ordebugVitePlugin
, something along these lines? Not sure how this works in other SDKs though, if we do the same there it's probably fine.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep a top-level debug option like this. Makes it very easy to ask users to enable debug mode to debug all issues related to build time stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about the same actually before creating this PR. But this debug option is actually quite nice for the build-time debug. The debug flag in
init
only works during runtime. To make it more explicit, this could be renamed tobuildDebug
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep it at
debug
tbhThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fwiw (no strong opinions but from having dealt with this before): We have this pattern of multiple
debug
properties in different files in SvelteKit and Astro and so far people haven't complained about it. If we point out well in the JSDoc what kind of logging this specific option enables, I think going simply withdebug
is fine.Especially considering, this flag can be used everywhere within the nuxt module (so not just within the vite plugin I assume) I wouldn't give it a too specific name.