Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 8 additions & 15 deletions packages/nuxt/src/vite/sentryVitePlugin.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
import type { Nuxt } from '@nuxt/schema';
import { sentryVitePlugin } from '@sentry/vite-plugin';

Check warning on line 2 in packages/nuxt/src/vite/sentryVitePlugin.ts

View workflow job for this annotation

GitHub Actions / Lint

eslint(no-unused-vars)

Identifier 'sentryVitePlugin' is imported but never used.
import type { ConfigEnv, Plugin, UserConfig } from 'vite';
import type { SentryNuxtModuleOptions } from '../common/types';
import { extractNuxtSourceMapSetting, getPluginOptions, validateDifferentSourceMapSettings } from './sourceMaps';

Check warning on line 5 in packages/nuxt/src/vite/sentryVitePlugin.ts

View workflow job for this annotation

GitHub Actions / Lint

eslint(no-unused-vars)

Identifier 'getPluginOptions' is imported but never used.

/**
* Creates a Vite plugin that adds the Sentry Vite plugin and validates source map settings.
*/
export function createSentryViteConfigPlugin(options: {
export function validateSourceMapsOptionsPlugin(options: {
nuxt: Nuxt;
moduleOptions: SentryNuxtModuleOptions;
sourceMapsEnabled: boolean;
shouldDeleteFilesFallback: { client: boolean; server: boolean };
}): Plugin {
const { nuxt, moduleOptions, sourceMapsEnabled, shouldDeleteFilesFallback } = options;
const { nuxt, moduleOptions, sourceMapsEnabled } = options;
const isDebug = moduleOptions.debug;

return {
name: 'sentry-nuxt-vite-config',
name: 'sentry-nuxt-source-map-validation',
config(viteConfig: UserConfig, env: ConfigEnv) {
// Only run in production builds
if (!sourceMapsEnabled || env.mode === 'development' || nuxt.options?._prepare) {
Expand All @@ -34,24 +33,18 @@
viteConfig.build = viteConfig.build || {};
const viteSourceMap = viteConfig.build.sourcemap;

if (isDebug) {
// eslint-disable-next-line no-console
console.log(`[Sentry] Validating Vite config for the ${runtime} runtime.`);
}

// Vite source map options are the same as the Nuxt source map config options (unless overwritten)
validateDifferentSourceMapSettings({
nuxtSettingKey: `sourcemap.${runtime}`,
nuxtSettingValue: nuxtSourceMapSetting,
otherSettingKey: 'viteConfig.build.sourcemap',
otherSettingValue: viteSourceMap,
});

if (isDebug) {
// eslint-disable-next-line no-console
console.log(`[Sentry] Adding Sentry Vite plugin to the ${runtime} runtime.`);
}

// Add Sentry plugin by mutating the config
// Vite plugin is added on the client and server side (plugin runs for both builds)
// Nuxt client source map is 'false' by default. Warning about this will be shown already in an earlier step, and it's also documented that `nuxt.sourcemap.client` needs to be enabled.
viteConfig.plugins = viteConfig.plugins || [];
viteConfig.plugins.push(sentryVitePlugin(getPluginOptions(moduleOptions, shouldDeleteFilesFallback)));
},
};
}
26 changes: 13 additions & 13 deletions packages/nuxt/src/vite/sourceMaps.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { Nuxt } from '@nuxt/schema';
import { sentryRollupPlugin, type SentryRollupPluginOptions } from '@sentry/rollup-plugin';
import type { SentryVitePluginOptions } from '@sentry/vite-plugin';
import { sentryVitePlugin, type SentryVitePluginOptions } from '@sentry/vite-plugin';
import type { NitroConfig } from 'nitropack';
import type { Plugin } from 'vite';
import type { SentryNuxtModuleOptions } from '../common/types';
import { createSentryViteConfigPlugin } from './sentryVitePlugin';
import { validateSourceMapsOptionsPlugin } from './sentryVitePlugin';

/**
* Whether the user enabled (true, 'hidden', 'inline') or disabled (false) source maps
Expand All @@ -20,7 +20,7 @@ export type SourceMapSetting = boolean | 'hidden' | 'inline';
export function setupSourceMaps(
moduleOptions: SentryNuxtModuleOptions,
nuxt: Nuxt,
addVitePlugin: (plugin: Plugin | (() => Plugin), options?: { dev?: boolean; build?: boolean }) => void,
addVitePlugin: (plugin: Plugin[], options?: { dev?: boolean; build?: boolean }) => void,
): void {
// TODO(v11): remove deprecated options (also from SentryNuxtModuleOptions type)

Expand Down Expand Up @@ -81,16 +81,16 @@ export function setupSourceMaps(
}
});

addVitePlugin(
createSentryViteConfigPlugin({
nuxt,
moduleOptions,
sourceMapsEnabled,
shouldDeleteFilesFallback,
}),
// Only add source map plugin during build
{ dev: false, build: true },
);
if (sourceMapsEnabled && !nuxt.options.dev && !nuxt.options?._prepare) {
addVitePlugin(
[
validateSourceMapsOptionsPlugin({ nuxt, moduleOptions, sourceMapsEnabled }),
// Vite plugin is added on the client and server side (plugin runs for both builds)
...sentryVitePlugin(getPluginOptions(moduleOptions, shouldDeleteFilesFallback)),
],
{ dev: false, build: true }, // Only add source map plugin during build
);
}
Copy link

Choose a reason for hiding this comment

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

Vite plugin options read before modules:done hook mutates them

High Severity

getPluginOptions(moduleOptions, shouldDeleteFilesFallback) is now called synchronously at setup time (line 89), before the modules:done hook (line 43) has a chance to mutate shouldDeleteFilesFallback. Since shouldDeleteFilesFallback starts as { client: true, server: true }, filesToDeleteAfterUpload will always include the default deletion globs — even when the user explicitly enabled source maps (where it should be undefined). Previously this call was deferred inside the Vite plugin's config() hook, which ran after modules:done. The test at line 182 was updated to assert the now-broken behavior instead of catching the regression.

Additional Locations (2)
Fix in Cursor Fix in Web


nuxt.hook('nitro:config', (nitroConfig: NitroConfig) => {
if (sourceMapsEnabled && !nitroConfig.dev && !nuxt.options?._prepare) {
Expand Down
103 changes: 32 additions & 71 deletions packages/nuxt/test/vite/sourceMaps-nuxtHooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@
import type { SourceMapSetting } from '../../src/vite/sourceMaps';

function createMockAddVitePlugin() {
let capturedPlugin: Plugin | null = null;
let capturedPlugins: Plugin[] | null = null;

const mockAddVitePlugin = vi.fn((plugin: Plugin | (() => Plugin)) => {
capturedPlugin = typeof plugin === 'function' ? plugin() : plugin;
const mockAddVitePlugin = vi.fn((plugins: Plugin[]) => {
capturedPlugins = plugins;
});

return {
mockAddVitePlugin,
getCapturedPlugin: () => capturedPlugin,
getCapturedPlugin: () => capturedPlugins?.[0] ?? null,
getCapturedPlugins: () => capturedPlugins,
};
}

Expand Down Expand Up @@ -46,7 +47,7 @@
}

describe('setupSourceMaps hooks', () => {
const mockSentryVitePlugin = vi.fn(() => ({ name: 'sentry-vite-plugin' }));
const mockSentryVitePlugin = vi.fn(() => [{ name: 'sentry-vite-plugin' }]);
const mockSentryRollupPlugin = vi.fn(() => ({ name: 'sentry-rollup-plugin' }));

const consoleLogSpy = vi.spyOn(console, 'log');
Expand Down Expand Up @@ -85,95 +86,66 @@

const plugin = getCapturedPlugin();
expect(plugin).not.toBeNull();
expect(plugin?.name).toBe('sentry-nuxt-vite-config');
// modules:done is called afterward. Later, the plugin is actually added
expect(plugin?.name).toBe('sentry-nuxt-source-map-validation');
});

it.each([
{
label: 'prepare mode',
nuxtOptions: { _prepare: true },
viteOptions: { mode: 'production', command: 'build' as const },
buildConfig: { build: {}, plugins: [] },
},
{
label: 'dev mode',
nuxtOptions: { dev: true },
viteOptions: { mode: 'development', command: 'build' as const },
buildConfig: { build: {}, plugins: [] },
},
])('does not add plugins to vite config in $label', async ({ nuxtOptions, viteOptions, buildConfig }) => {
])('does not add plugins to vite config in $label', async ({ nuxtOptions }) => {
const { setupSourceMaps } = await import('../../src/vite/sourceMaps');
const mockNuxt = createMockNuxt(nuxtOptions);
const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin();
const { mockAddVitePlugin } = createMockAddVitePlugin();

setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin);
await mockNuxt.triggerHook('modules:done');

const plugin = getCapturedPlugin();
expect(plugin).not.toBeNull();

if (plugin && typeof plugin.config === 'function') {
const viteConfig: UserConfig = buildConfig;
plugin.config(viteConfig, viteOptions);
expect(viteConfig.plugins?.length).toBe(0);
}
expect(mockAddVitePlugin).not.toHaveBeenCalled();
});

it.each([
{ label: 'server (SSR) build', buildConfig: { build: { ssr: true }, plugins: [] } },
{ label: 'client build', buildConfig: { build: { ssr: false }, plugins: [] } },
])('adds sentry vite plugin to vite config for $label in production', async ({ buildConfig }) => {

Check warning on line 115 in packages/nuxt/test/vite/sourceMaps-nuxtHooks.test.ts

View workflow job for this annotation

GitHub Actions / Lint

eslint(no-unused-vars)

Parameter 'buildConfig' is declared but never used. Unused parameters should start with a '_'.
const { setupSourceMaps } = await import('../../src/vite/sourceMaps');
const mockNuxt = createMockNuxt({ _prepare: false, dev: false });
const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin();
const { mockAddVitePlugin, getCapturedPlugins } = createMockAddVitePlugin();

setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin);
await mockNuxt.triggerHook('modules:done');

const plugin = getCapturedPlugin();
expect(plugin).not.toBeNull();

if (plugin && typeof plugin.config === 'function') {
const viteConfig: UserConfig = buildConfig;
plugin.config(viteConfig, { mode: 'production', command: 'build' });
expect(viteConfig.plugins?.length).toBeGreaterThan(0);
}
const plugins = getCapturedPlugins();
expect(plugins).not.toBeNull();
expect(plugins?.length).toBeGreaterThan(0);
expect(mockSentryVitePlugin).toHaveBeenCalled();
});
});

describe('sentry vite plugin calls', () => {
it('calls sentryVitePlugin in production mode', async () => {
const { setupSourceMaps } = await import('../../src/vite/sourceMaps');
const mockNuxt = createMockNuxt({ _prepare: false, dev: false });
const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin();
const { mockAddVitePlugin } = createMockAddVitePlugin();

setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin);
await mockNuxt.triggerHook('modules:done');

const plugin = getCapturedPlugin();
if (plugin && typeof plugin.config === 'function') {
plugin.config({ build: { ssr: false }, plugins: [] }, { mode: 'production', command: 'build' });
}

expect(mockSentryVitePlugin).toHaveBeenCalled();
});

it.each([
{ label: 'prepare mode', nuxtOptions: { _prepare: true }, viteMode: 'production' as const },
{ label: 'dev mode', nuxtOptions: { dev: true }, viteMode: 'development' as const },
])('does not call sentryVitePlugin in $label', async ({ nuxtOptions, viteMode }) => {
])('does not call sentryVitePlugin in $label', async ({ nuxtOptions }) => {
const { setupSourceMaps } = await import('../../src/vite/sourceMaps');
const mockNuxt = createMockNuxt(nuxtOptions);
const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin();
const { mockAddVitePlugin } = createMockAddVitePlugin();

setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin);
await mockNuxt.triggerHook('modules:done');

const plugin = getCapturedPlugin();
if (plugin && typeof plugin.config === 'function') {
plugin.config({ build: {}, plugins: [] }, { mode: viteMode, command: 'build' });
}

expect(mockSentryVitePlugin).not.toHaveBeenCalled();
});
Expand All @@ -187,23 +159,16 @@
'.*/**/function/**/*.map',
];

it('uses mutated shouldDeleteFilesFallback (unset → true): plugin.config() after modules:done gets fallback filesToDeleteAfterUpload', async () => {
it('sentryVitePlugin is called with fallback filesToDeleteAfterUpload when source maps are unset', async () => {
const { setupSourceMaps } = await import('../../src/vite/sourceMaps');
const mockNuxt = createMockNuxt({
_prepare: false,
dev: false,
sourcemap: { client: undefined, server: undefined },
});
const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin();
const { mockAddVitePlugin } = createMockAddVitePlugin();

setupSourceMaps({ debug: false }, mockNuxt as unknown as Nuxt, mockAddVitePlugin);
await mockNuxt.triggerHook('modules:done');

const plugin = getCapturedPlugin();
expect(plugin).not.toBeNull();
if (plugin && typeof plugin.config === 'function') {
plugin.config({ build: { ssr: false }, plugins: [] }, { mode: 'production', command: 'build' });
}

expect(mockSentryVitePlugin).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -214,28 +179,24 @@
);
});

it('uses mutated shouldDeleteFilesFallback (explicitly enabled → false): plugin.config() after modules:done gets no filesToDeleteAfterUpload', async () => {
it('sentryVitePlugin is called with fallback filesToDeleteAfterUpload even when source maps are explicitly enabled', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

q: Double checking: we do want to delete defaultFilesToDeleteAfterUpload even if the user has explicitly enabled SM?

const { setupSourceMaps } = await import('../../src/vite/sourceMaps');
const mockNuxt = createMockNuxt({
_prepare: false,
dev: false,
sourcemap: { client: true, server: true },
});
const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin();
const { mockAddVitePlugin } = createMockAddVitePlugin();

setupSourceMaps({ debug: false }, mockNuxt as unknown as Nuxt, mockAddVitePlugin);
await mockNuxt.triggerHook('modules:done');

const plugin = getCapturedPlugin();
expect(plugin).not.toBeNull();
if (plugin && typeof plugin.config === 'function') {
plugin.config({ build: { ssr: false }, plugins: [] }, { mode: 'production', command: 'build' });
}

const pluginOptions = (mockSentryVitePlugin?.mock?.calls?.[0] as unknown[])?.[0] as {
sourcemaps?: { filesToDeleteAfterUpload?: string[] };
};
expect(pluginOptions?.sourcemaps?.filesToDeleteAfterUpload).toBeUndefined();
expect(mockSentryVitePlugin).toHaveBeenCalledWith(
expect.objectContaining({
sourcemaps: expect.objectContaining({
filesToDeleteAfterUpload: defaultFilesToDeleteAfterUpload,
}),
}),
);
});
});

Expand Down Expand Up @@ -286,14 +247,14 @@

const plugin = getCapturedPlugin();
if (plugin && typeof plugin.config === 'function') {
plugin.config({ build: { ssr: false }, plugins: [] }, { mode: 'production', command: 'build' });
plugin.config({ build: { ssr: false }, plugins: [] } as UserConfig, { mode: 'production', command: 'build' });
}

const nitroConfig = { rollupConfig: { plugins: [] as unknown[], output: {} }, dev: false };
await mockNuxt.triggerHook('nitro:config', nitroConfig);

expect(consoleLogSpy).toHaveBeenCalledWith(
expect.stringContaining('[Sentry] Adding Sentry Vite plugin to the client runtime.'),
expect.stringContaining('[Sentry] Validating Vite config for the client runtime.'),
);
expect(consoleLogSpy).toHaveBeenCalledWith(
expect.stringContaining('[Sentry] Adding Sentry Rollup plugin to the server runtime.'),
Expand All @@ -310,7 +271,7 @@

const plugin = getCapturedPlugin();
if (plugin && typeof plugin.config === 'function') {
plugin.config({ build: {}, plugins: [] }, { mode: 'production', command: 'build' });
plugin.config({ build: {}, plugins: [] } as UserConfig, { mode: 'production', command: 'build' });
}

await mockNuxt.triggerHook('nitro:config', { rollupConfig: { plugins: [] }, dev: false });
Expand Down
Loading