feat(sveltekit): Respect user-provided source map generation settings

packages/solidstart/src/vite/sourceMaps.ts

@@ -18,7 +18,7 @@ export function makeAddSentryVitePlugin(options: SentrySolidStartPluginOptions,
     // Only if source maps were previously not set, we update the "filesToDeleteAfterUpload" (as we override the setting with "hidden")
     typeof viteConfig.build?.sourcemap === 'undefined'
   ) {
-    // This also works for adapters, as the source maps are also copied to e.g. the .vercel folder
+    // For .output, .vercel, .netlify etc.
     updatedFilesToDeleteAfterUpload = ['.*/**/*.map'];
 
     consoleSandbox(() => {

packages/sveltekit/src/vite/sourceMaps.ts

@@ -1,11 +1,12 @@
+/* eslint-disable max-lines */
 import * as child_process from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
-import { escapeStringForRegex, uuid4 } from '@sentry/core';
+import { consoleSandbox, escapeStringForRegex, uuid4 } from '@sentry/core';
 import { getSentryRelease } from '@sentry/node';
 import type { SentryVitePluginOptions } from '@sentry/vite-plugin';
 import { sentryVitePlugin } from '@sentry/vite-plugin';
-import type { Plugin } from 'vite';
+import { type Plugin, type UserConfig, loadConfigFromFile } from 'vite';
 
 import MagicString from 'magic-string';
 import { WRAPPED_MODULE_SUFFIX } from './autoInstrument';
@@ -27,6 +28,18 @@ type Sorcery = {
 // and we only want to generate a uuid once in case we have to fall back to it.
 const releaseName = detectSentryRelease();
 
+let sourceMapSetting: {
+  updatedSourceMapSetting?: boolean | 'inline' | 'hidden';
+  previousSourceMapSetting?: UserSourceMapSetting;
+} = { previousSourceMapSetting: undefined, updatedSourceMapSetting: undefined };
+
+/**
+ * For mocking the value of `sourceMapSetting` in tests
+ */
+export const __setSourceMapSettingForTest = (value: typeof sourceMapSetting): void => {
+  sourceMapSetting = value;
+};
+
 /**
  * Creates a new Vite plugin that uses the unplugin-based Sentry Vite plugin to create
  * releases and upload source maps to Sentry.
@@ -60,14 +73,50 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
     },
   };
 
+  const filesToDeleteGlob = './.*/**/*.map';
+
+  if (sourceMapSetting.updatedSourceMapSetting === undefined) {
+    const configFile = await loadConfigFromFile({ command: 'build', mode: 'production' });
+
+    if (configFile) {
+      sourceMapSetting = getUpdatedSourceMapSetting(configFile.config);
+    } else {
+      if (options?.debug) {
+        consoleSandbox(() => {
+          // eslint-disable-next-line no-console
+          console.warn(
+            '[Source Maps Plugin] Could not load Vite config with Vite "production" mode. This is needed for Sentry to automatically update source map settings.',
+          );
+        });
+      }
+    }
+
+    if (options?.debug && sourceMapSetting.previousSourceMapSetting === 'unset') {
+      consoleSandbox(() => {
+        // eslint-disable-next-line no-console
+        console.warn(
+          `[Source Maps Plugin] Automatically setting \`sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload: ["${filesToDeleteGlob}"]\` to delete generated source maps after they were uploaded to Sentry.`,
+        );
+      });
+    }
+  }
+
   const mergedOptions = {
     ...defaultPluginOptions,
     ...options,
     release: {
       ...defaultPluginOptions.release,
       ...options?.release,
     },
+    sourcemaps: {
+      ...options?.sourcemaps,
+      filesToDeleteAfterUpload:
+        sourceMapSetting.previousSourceMapSetting === 'unset' && !options?.sourcemaps?.filesToDeleteAfterUpload
+          ? ['./.*/**/*.map']
+          : options?.sourcemaps?.filesToDeleteAfterUpload,
+    },
   };
+
   const { debug } = mergedOptions;
 
   const sentryPlugins: Plugin[] = await sentryVitePlugin(mergedOptions);
@@ -129,34 +178,48 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
     __sentry_sveltekit_output_dir: outputDir,
   };
 
-  const customDebugIdUploadPlugin: Plugin = {
-    name: 'sentry-sveltekit-debug-id-upload-plugin',
+  const sourceMapSettingsPlugin: Plugin = {
+    name: 'sentry-sveltekit-update-source-map-setting-plugin',
     apply: 'build', // only apply this plugin at build time
-    enforce: 'post', // this needs to be set to post, otherwise we don't pick up the output from the SvelteKit adapter
+    config: (config: UserConfig) => {
+      const settingKey = 'build.sourcemap';
 
-    // Modify the config to generate source maps
-    config: config => {
-      const sourceMapsPreviouslyNotEnabled = !config.build?.sourcemap;
-      if (debug && sourceMapsPreviouslyNotEnabled) {
-        // eslint-disable-next-line no-console
-        console.log('[Source Maps Plugin] Enabling source map generation');
-        if (!mergedOptions.sourcemaps?.filesToDeleteAfterUpload) {
-          // eslint-disable-next-line no-console
+      if (sourceMapSetting.previousSourceMapSetting === 'unset') {
+        consoleSandbox(() => {
+          //  eslint-disable-next-line no-console
+          console.log(`[Sentry] Enabled source map generation in the build options with \`${settingKey}: "hidden"\`.`);
+        });
+
+        return {
+          ...config,
+          build: { ...config.build, sourcemap: 'hidden' },
+        };
+      } else if (sourceMapSetting.previousSourceMapSetting === 'disabled') {
+        consoleSandbox(() => {
+          //  eslint-disable-next-line no-console
           console.warn(
-            `[Source Maps Plugin] We recommend setting the \`sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload\` option to clean up source maps after uploading.
-[Source Maps Plugin] Otherwise, source maps might be deployed to production, depending on your configuration`,
+            `[Sentry] Parts of source map generation are currently disabled in your Vite configuration (\`${settingKey}: false\`). This setting is either a default setting or was explicitly set in your configuration. Sentry won't override this setting. Without source maps, code snippets on the Sentry Issues page will remain minified. To show unminified code, enable source maps in \`${settingKey}\` (e.g. by setting them to \`hidden\`).`,
           );
+        });
+      } else if (sourceMapSetting.previousSourceMapSetting === 'enabled') {
+        if (mergedOptions?.debug) {
+          consoleSandbox(() => {
+            // eslint-disable-next-line no-console
+            console.log(
+              `[Sentry] We discovered you enabled source map generation in  your Vite configuration (\`${settingKey}\`). Sentry will keep this source map setting. This will un-minify the code snippet on the Sentry Issue page.`,
+            );
+          });
         }
       }
-      return {
-        ...config,
-        build: {
-          ...config.build,
-          sourcemap: true,
-        },
-      };
+
+      return config;
     },
+  };
 
+  const customDebugIdUploadPlugin: Plugin = {
+    name: 'sentry-sveltekit-debug-id-upload-plugin',
+    apply: 'build', // only apply this plugin at build time
+    enforce: 'post', // this needs to be set to post, otherwise we don't pick up the output from the SvelteKit adapter
     resolveId: (id, _importer, _ref) => {
       if (id === VIRTUAL_GLOBAL_VALUES_FILE) {
         return {
@@ -326,12 +389,59 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
 
   return [
     ...unchangedSentryVitePlugins,
+    sourceMapSettingsPlugin,
     customReleaseManagementPlugin,
     customDebugIdUploadPlugin,
     customFileDeletionPlugin,
   ];
 }
 
+/**
+ * Whether the user enabled (true, 'hidden', 'inline') or disabled (false) source maps
+ */
+export type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined;
+
+/** There are 3 ways to set up source map generation (https://github.com/getsentry/sentry-javascript/issues/13993)
+ *
+ *     1. User explicitly disabled source maps
+ *       - keep this setting (emit a warning that errors won't be unminified in Sentry)
+ *       - We won't upload anything
+ *
+ *     2. Users enabled source map generation (true, 'hidden', 'inline').
+ *       - keep this setting (don't do anything - like deletion - besides uploading)
+ *
+ *     3. Users didn't set source maps generation
+ *       - we enable 'hidden' source maps generation
+ *       - configure `filesToDeleteAfterUpload` to delete all .map files (we emit a log about this)
+ *
+ * --> only exported for testing
+ */
+export function getUpdatedSourceMapSetting(viteConfig: {
+  build?: {
+    sourcemap?: boolean | 'inline' | 'hidden';
+  };
+}): { updatedSourceMapSetting: boolean | 'inline' | 'hidden'; previousSourceMapSetting: UserSourceMapSetting } {
+  let previousSourceMapSetting: UserSourceMapSetting;
+  let updatedSourceMapSetting: boolean | 'inline' | 'hidden' | undefined;
+
+  viteConfig.build = viteConfig.build || {};
+
+  const viteSourceMap = viteConfig.build.sourcemap;
+
+  if (viteSourceMap === false) {
+    previousSourceMapSetting = 'disabled';
+    updatedSourceMapSetting = viteSourceMap;
+  } else if (viteSourceMap && ['hidden', 'inline', true].includes(viteSourceMap)) {
+    previousSourceMapSetting = 'enabled';
+    updatedSourceMapSetting = viteSourceMap;
+  } else {
+    previousSourceMapSetting = 'unset';
+    updatedSourceMapSetting = 'hidden';
+  }
+
+  return { previousSourceMapSetting, updatedSourceMapSetting };
+}
+
 function getFiles(dir: string): string[] {
   if (!fs.existsSync(dir)) {
     return [];

packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts

@@ -43,7 +43,7 @@ describe('sentrySvelteKit()', () => {
 
     expect(plugins).toBeInstanceOf(Array);
     // 1 auto instrument plugin + 5 source maps plugins
-    expect(plugins).toHaveLength(7);
+    expect(plugins).toHaveLength(8);
   });
 
   it('returns the custom sentry source maps upload plugin, unmodified sourcemaps plugins and the auto-instrument plugin by default', async () => {
@@ -56,6 +56,7 @@ describe('sentrySvelteKit()', () => {
       'sentry-telemetry-plugin',
       'sentry-vite-release-injection-plugin',
       'sentry-vite-debug-id-injection-plugin',
+      'sentry-sveltekit-update-source-map-setting-plugin',
       // custom release plugin:
       'sentry-sveltekit-release-management-plugin',
       // custom source maps plugin:
@@ -86,7 +87,7 @@ describe('sentrySvelteKit()', () => {
   it("doesn't return the auto instrument plugin if autoInstrument is `false`", async () => {
     const plugins = await getSentrySvelteKitPlugins({ autoInstrument: false });
     const pluginNames = plugins.map(plugin => plugin.name);
-    expect(plugins).toHaveLength(6);
+    expect(plugins).toHaveLength(7);
     expect(pluginNames).not.toContain('sentry-upload-source-maps');
   });
 

packages/sveltekit/test/vite/sourceMaps.test.ts

@@ -1,4 +1,5 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { __setSourceMapSettingForTest, getUpdatedSourceMapSetting } from '../../src/vite/sourceMaps';
 
 import type { Plugin } from 'vite';
 import { makeCustomSentryVitePlugins } from '../../src/vite/sourceMaps';
@@ -55,7 +56,7 @@ async function getSentryViteSubPlugin(name: string): Promise<Plugin | undefined>
   return plugins.find(plugin => plugin.name === name);
 }
 
-describe('makeCustomSentryVitePlugin()', () => {
+describe('makeCustomSentryVitePlugins()', () => {
   it('returns the custom sentry source maps plugin', async () => {
     const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');
 
@@ -66,20 +67,36 @@ describe('makeCustomSentryVitePlugin()', () => {
     expect(plugin?.resolveId).toBeInstanceOf(Function);
     expect(plugin?.transform).toBeInstanceOf(Function);
 
-    expect(plugin?.config).toBeInstanceOf(Function);
     expect(plugin?.configResolved).toBeInstanceOf(Function);
 
     // instead of writeBundle, this plugin uses closeBundle
     expect(plugin?.closeBundle).toBeInstanceOf(Function);
     expect(plugin?.writeBundle).toBeUndefined();
   });
 
-  describe('Custom debug id source maps plugin plugin', () => {
-    it('enables source map generation', async () => {
-      const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');
+  describe('Custom source map settings update plugin', () => {
+    it('returns the custom sentry source maps plugin', async () => {
+      const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin');
+
+      expect(plugin?.name).toEqual('sentry-sveltekit-update-source-map-setting-plugin');
+      expect(plugin?.apply).toEqual('build');
+      expect(plugin?.config).toBeInstanceOf(Function);
+    });
+
+    it('keeps source map generation settings when previously enabled', async () => {
+      const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin');
+
+      __setSourceMapSettingForTest({
+        previousSourceMapSetting: 'enabled',
+        updatedSourceMapSetting: undefined,
+      });
+
       // @ts-expect-error this function exists!
-      const sentrifiedConfig = plugin.config({ build: { foo: {} }, test: {} });
-      expect(sentrifiedConfig).toEqual({
+      const sentryConfig = plugin.config({
+        build: { sourcemap: true, foo: {} },
+        test: {},
+      });
+      expect(sentryConfig).toEqual({
         build: {
           foo: {},
           sourcemap: true,
@@ -88,6 +105,43 @@ describe('makeCustomSentryVitePlugin()', () => {
       });
     });
 
+    it('keeps source map generation settings when previously disabled', async () => {
+      const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin');
+
+      __setSourceMapSettingForTest({
+        previousSourceMapSetting: 'disabled',
+        updatedSourceMapSetting: undefined,
+      });
+
+      // @ts-expect-error this function exists!
+      const sentryConfig = plugin.config({
+        build: { sourcemap: false, foo: {} },
+        test: {},
+      });
+      expect(sentryConfig).toEqual({
+        build: {
+          foo: {},
+          sourcemap: false,
+        },
+        test: {},
+      });
+    });
+
+    it('enables source map generation with "hidden" when unset', async () => {
+      const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin');
+      // @ts-expect-error this function exists!
+      const sentryConfig = plugin.config({ build: { foo: {} }, test: {} });
+      expect(sentryConfig).toEqual({
+        build: {
+          foo: {},
+          sourcemap: 'hidden',
+        },
+        test: {},
+      });
+    });
+  });
+
+  describe('Custom debug id source maps plugin plugin', () => {
     it('injects the output dir into the server hooks file', async () => {
       const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');
       // @ts-expect-error this function exists!
@@ -237,3 +291,25 @@ describe('makeCustomSentryVitePlugin()', () => {
     });
   });
 });
+
+describe('changeViteSourceMapSettings()', () => {
+  it('handles vite source map settings', () => {
+    const cases = [
+      { sourcemap: false, expectedSourcemap: false, expectedPrevious: 'disabled' },
+      { sourcemap: 'hidden', expectedSourcemap: 'hidden', expectedPrevious: 'enabled' },
+      { sourcemap: 'inline', expectedSourcemap: 'inline', expectedPrevious: 'enabled' },
+      { sourcemap: true, expectedSourcemap: true, expectedPrevious: 'enabled' },
+      { sourcemap: undefined, expectedSourcemap: 'hidden', expectedPrevious: 'unset' },
+    ];
+
+    cases.forEach(({ sourcemap, expectedSourcemap, expectedPrevious }) => {
+      const viteConfig = { build: { sourcemap } };
+      const result = getUpdatedSourceMapSetting(viteConfig);
+
+      expect(result).toEqual({
+        updatedSourceMapSetting: expectedSourcemap,
+        previousSourceMapSetting: expectedPrevious,
+      });
+    });
+  });
+});