Skip to content

feat(nuxt): Setup source maps with vite config #13018

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

Merged
merged 6 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/astro/src/integration/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type SourceMapsOptions = {
telemetry?: boolean;

/**
* A glob or an array of globs that specify the build artifacts and source maps that will uploaded to Sentry.
* A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry.
*
* If this option is not specified, sensible defaults based on your `outDir`, `rootDir` and `adapter`
* config will be used. Use this option to override these defaults, for instance if you have a
Expand Down
1 change: 1 addition & 0 deletions packages/nuxt/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module.exports = {
files: ['src/vite/**', 'src/server/**'],
rules: {
'@sentry-internal/sdk/no-optional-chaining': 'off',
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
},
},
],
Expand Down
22 changes: 17 additions & 5 deletions packages/nuxt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,24 @@ Add an import flag to the node options, so the file loads before any other impor
}
```

### 5. Vite Setup

todo: add vite setup

---

## Uploading Source Maps

todo: add source maps instructions
To upload source maps, you can use the `sourceMapsUploadOptions` option inside the `sentry` options of your
`nuxt.config.ts`:

```javascript
// nuxt.config.ts
export default defineNuxtConfig({
modules: ['@sentry/nuxt/module'],
sentry: {
debug: true,
Copy link
Member

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 or debugVitePlugin, something along these lines? Not sure how this works in other SDKs though, if we do the same there it's probably fine.

Copy link
Contributor

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.

Copy link
Member Author

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 to buildDebug?

Copy link
Contributor

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 tbh

Copy link
Member

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 with debug 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.

sourceMapsUploadOptions: {
org: 'your-org-slug',
project: 'your-project-slug',
authToken: process.env.SENTRY_AUTH_TOKEN,
},
},
});
```
67 changes: 67 additions & 0 deletions packages/nuxt/src/common/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,71 @@
import type { Options } from '@sentry/types';
import type { init } from '@sentry/vue';

// Omitting 'app' as the Nuxt SDK will add the app instance in the client plugin (users do not have to provide this)
export type SentryNuxtOptions = Omit<Parameters<typeof init>[0] & object, 'app'>;

type SourceMapsOptions = {
/**
* Options for the Sentry Vite plugin to customize the source maps upload process.
*
* These options are always read from the `sentry` module options in the `nuxt.config.(js|ts).
* Do not define them in the `sentry.client.config.(js|ts)` or `sentry.server.config.(js|ts)` files.
*/
sourceMapsUploadOptions?: {
/**
* If this flag is `true`, and an auth token is detected, the Sentry integration will
* automatically generate and upload source maps to Sentry during a production build.
*
* @default true
*/
enabled?: boolean;

/**
* The auth token to use when uploading source maps to Sentry.
*
* Instead of specifying this option, you can also set the `SENTRY_AUTH_TOKEN` environment variable.
*
* To create an auth token, follow this guide:
* @see https://docs.sentry.io/product/accounts/auth-tokens/#organization-auth-tokens
*/
authToken?: string;

/**
* The organization slug of your Sentry organization.
* Instead of specifying this option, you can also set the `SENTRY_ORG` environment variable.
*/
org?: string;

/**
* The project slug of your Sentry project.
* Instead of specifying this option, you can also set the `SENTRY_PROJECT` environment variable.
*/
project?: string;

/**
* If this flag is `true`, the Sentry plugin will collect some telemetry data and send it to Sentry.
* It will not collect any sensitive or user-specific data.
*
* @default true
*/
telemetry?: boolean;

/**
* A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry.
*
* If this option is not specified, sensible defaults based on your `outDir`, `rootDir` and `adapter`
* config will be used. Use this option to override these defaults, for instance if you have a
* customized build setup that diverges from Nuxt's defaults.
*
* The globbing patterns must follow the implementation of the `glob` package.
* @see https://www.npmjs.com/package/glob#glob-primer
*/
assets?: string | Array<string>;
};
};

/**
* The SDK options are mostly handled inside the `init` function in separate files (see type `SentryNuxtOptions`).
* Other options, such as the source maps options are added inside the `nuxt.config.ts` to be able to access those options during build time and modify the Vite config.
*/
export type SentryNuxtModuleOptions = Pick<Options, 'debug'> & SourceMapsOptions;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Any reason you picked debug off the SDK options? I would just add a new debug field and document it separately, because it has nothing to do with the SDK init debug option.
  • I would phrase the JS doc for this type differently. Imagine you are a user hovering over the type. I would start with something like "Build options used by the Sentry Nuxt SDK. ...". In general I would steer away from describing how things are not and describe what they are.
  • I would restructure this type and the SourceMapsOptions type so that it is not just an interface with one property.

11 changes: 8 additions & 3 deletions packages/nuxt/src/module.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import * as fs from 'fs';
import * as path from 'path';
import { addPlugin, addPluginTemplate, addServerPlugin, createResolver, defineNuxtModule } from '@nuxt/kit';
import type { SentryNuxtOptions } from './common/types';
import type { SentryNuxtModuleOptions } from './common/types';
import { setupSourceMaps } from './vite/sourceMaps';

export type ModuleOptions = SentryNuxtOptions;
export type ModuleOptions = SentryNuxtModuleOptions;

export default defineNuxtModule<ModuleOptions>({
meta: {
Expand All @@ -14,7 +15,7 @@ export default defineNuxtModule<ModuleOptions>({
},
},
defaults: {},
setup(_moduleOptions, nuxt) {
setup(moduleOptions, nuxt) {
const moduleDirResolver = createResolver(import.meta.url);
const buildDirResolver = createResolver(nuxt.options.buildDir);

Expand Down Expand Up @@ -47,6 +48,10 @@ export default defineNuxtModule<ModuleOptions>({

addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server'));
}

if (clientConfigFile || serverConfigFile) {
setupSourceMaps(moduleOptions, nuxt);
}
},
});

Expand Down
33 changes: 33 additions & 0 deletions packages/nuxt/src/vite/sourceMaps.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
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 sentryPlugins = sentryVitePlugin({
org: sourceMapsUploadOptions.org ?? process.env.SENTRY_ORG,
Copy link
Member

Choose a reason for hiding this comment

The 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,
_metaOptions: {
telemetry: {
metaFramework: 'nuxt',
},
},
debug: moduleOptions.debug ?? false,
});

viteInlineConfig.plugins = viteInlineConfig.plugins || [];
viteInlineConfig.plugins.push(...sentryPlugins);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should be able to do

Suggested change
viteInlineConfig.plugins.push(...sentryPlugins);
viteInlineConfig.plugins.push(sentryPlugins);

and I highly recommend because you theoretically can not assume that the vite plugin is an array.


viteInlineConfig.build = viteInlineConfig.build || {};
viteInlineConfig.build.sourcemap = true;
Copy link
Member

Choose a reason for hiding this comment

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

m: If viteInlineConfig.build.sourcemap wasn't true before we set it, I'd recommend to log out a warning that we're changing their build config to enable emitting source maps. It's fine to do that but users are worried that their source maps then get deployed to prod.
I'd recommend we expose the filesToDeleteAfterUpload option from the plugin and hint them in the warning that they should set it if they want to delete source maps afterwards again.

}
});
}
Loading