diff --git a/dev-packages/e2e-tests/test-applications/solidstart-2/assert-build.ts b/dev-packages/e2e-tests/test-applications/solidstart-2/assert-build.ts index 9cf7afcbe4ec..aa917dce5157 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart-2/assert-build.ts +++ b/dev-packages/e2e-tests/test-applications/solidstart-2/assert-build.ts @@ -1,24 +1,37 @@ +import * as assert from 'node:assert/strict'; import * as fs from 'node:fs'; import * as path from 'node:path'; -import { assertSourceMapUpload, findDebugIdsInFilesWithGlobCharacters } from '@sentry-internal/test-utils'; +import { + assertSourceMapUpload, + findDebugIdsInFilesWithGlobCharacters, + findSourceMapFiles, + findSourceMappingUrlComments, +} from '@sentry-internal/test-utils'; const CLIENT_OUTPUT = path.join('.output', 'public'); const SERVER_OUTPUT = path.join('.output', 'server'); -// The app does not set `build.sourcemap`, so the SDK enables hidden maps and must delete them after upload. -// TODO: The client maps stay in `.output/public`, because the SDK's deletion glob `./**/*.map` skips dot -// folders. Add `CLIENT_OUTPUT` to `strippedDirs` once the SDK deletes them. -assertSourceMapUpload({ - outputDirs: [CLIENT_OUTPUT, SERVER_OUTPUT], - strippedDirs: [SERVER_OUTPUT], - markers: { 'client source': 'SOURCEMAP_MARKER_CLIENT', 'server source': 'SOURCEMAP_MARKER_SERVER' }, - debugIdsWithoutSourceMap: [ - // Written by the build, see `recordChunksWithoutSourceMap` in `vite.config.ts`. - ...(fs.existsSync('.tmp_debug_ids_without_source_map') - ? fs.readFileSync('.tmp_debug_ids_without_source_map', 'utf8').split('\n').filter(Boolean) - : []), - ...findDebugIdsInFilesWithGlobCharacters({ outputDir: SERVER_OUTPUT }), - ], -}); +// The app does not set `build.sourcemap`, so the SDK enables hidden maps and asks the bundler plugin to +// remove them from the client output, also when no upload runs. `@sentry/nitro` handles the server output. +if (process.env.E2E_NO_UPLOAD) { + assert.deepEqual(findSourceMapFiles({ outputDir: CLIENT_OUTPUT }), [], `Expected no source maps in ${CLIENT_OUTPUT}`); + assert.deepEqual( + findSourceMappingUrlComments({ outputDir: CLIENT_OUTPUT }), + [], + `Expected no sourceMappingURL comments in ${CLIENT_OUTPUT}`, + ); +} else { + assertSourceMapUpload({ + outputDirs: [CLIENT_OUTPUT, SERVER_OUTPUT], + markers: { 'client source': 'SOURCEMAP_MARKER_CLIENT', 'server source': 'SOURCEMAP_MARKER_SERVER' }, + debugIdsWithoutSourceMap: [ + // Written by the build, see `recordChunksWithoutSourceMap` in `vite.config.ts`. + ...(fs.existsSync('.tmp_debug_ids_without_source_map') + ? fs.readFileSync('.tmp_debug_ids_without_source_map', 'utf8').split('\n').filter(Boolean) + : []), + ...findDebugIdsInFilesWithGlobCharacters({ outputDir: SERVER_OUTPUT }), + ], + }); +} console.log('All sourcemap assertions passed!'); diff --git a/dev-packages/e2e-tests/test-applications/solidstart-2/package.json b/dev-packages/e2e-tests/test-applications/solidstart-2/package.json index 7794cea772f2..b075dcec57d5 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart-2/package.json +++ b/dev-packages/e2e-tests/test-applications/solidstart-2/package.json @@ -10,6 +10,8 @@ "test:prod": "TEST_ENV=production playwright test", "test:build": "pnpm install && pnpm build", "test:assert": "node assert-build.ts && pnpm test:prod", + "test:build:no-upload": "pnpm install && E2E_NO_UPLOAD=1 pnpm build", + "test:assert:no-upload": "E2E_NO_UPLOAD=1 node assert-build.ts", "test:build:tunnel-basepath": "pnpm install && E2E_TEST_TUNNEL=1 E2E_TEST_BASE_PATH=/app pnpm build", "test:assert:tunnel-basepath": "E2E_TEST_TUNNEL=1 E2E_TEST_BASE_PATH=/app E2E_TEST_DSN=http://public@localhost:3031/1337 pnpm test:prod tunnel", "test:assert:tunnel-basepath-dev": "E2E_TEST_TUNNEL=1 E2E_TEST_BASE_PATH=/app E2E_TEST_DSN=http://public@localhost:3031/1337 TEST_ENV=development playwright test tunnel" @@ -36,6 +38,11 @@ "sentryTest": { "optional": true, "variants": [ + { + "build-command": "pnpm test:build:no-upload", + "assert-command": "pnpm test:assert:no-upload", + "label": "solidstart-2 (no upload)" + }, { "build-command": "pnpm test:build:tunnel-basepath", "assert-command": "pnpm test:assert:tunnel-basepath", diff --git a/dev-packages/e2e-tests/test-applications/solidstart-2/vite.config.ts b/dev-packages/e2e-tests/test-applications/solidstart-2/vite.config.ts index 24cb35bad781..fd6b29c1edac 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart-2/vite.config.ts +++ b/dev-packages/e2e-tests/test-applications/solidstart-2/vite.config.ts @@ -5,21 +5,24 @@ import { defineConfig } from 'vite'; import * as fs from 'node:fs'; // Rolldown emits no map for chunks without mappable source (e.g. Vite's preload helper), so their debug -// IDs can't be uploaded. Recorded here because the SDK deletes all maps after the upload. +// IDs can't be uploaded. Recorded before the bundler plugin removes the maps from the bundle. function recordChunksWithoutSourceMap() { return { name: 'record-chunks-without-source-map', apply: 'build' as const, enforce: 'post' as const, - writeBundle(_options: unknown, bundle: Record) { - for (const output of Object.values(bundle)) { - if (output.type !== 'chunk' || !output.code || bundle[`${output.fileName}.map`]) { - continue; + generateBundle: { + order: 'pre' as const, + handler(_options: unknown, bundle: Record) { + for (const output of Object.values(bundle)) { + if (output.type !== 'chunk' || !output.code || bundle[`${output.fileName}.map`]) { + continue; + } + for (const match of output.code.matchAll(/sentry-dbid-([\da-f-]{36})/gi)) { + fs.appendFileSync('.tmp_debug_ids_without_source_map', `${match[1]!.toLowerCase()}\n`); + } } - for (const match of output.code.matchAll(/sentry-dbid-([\da-f-]{36})/gi)) { - fs.appendFileSync('.tmp_debug_ids_without_source_map', `${match[1]!.toLowerCase()}\n`); - } - } + }, }, }; } @@ -37,7 +40,7 @@ export default defineConfig({ }), sentrySolidStart({ sentryUrl: 'http://localhost:3032', - authToken: 'fake-auth-token', + authToken: process.env.E2E_NO_UPLOAD ? undefined : 'fake-auth-token', org: 'test-org', project: 'test-project', release: { name: 'test-release' }, diff --git a/dev-packages/e2e-tests/test-applications/solidstart/app.config.ts b/dev-packages/e2e-tests/test-applications/solidstart/app.config.ts index fb663eb94953..1af6508ddc36 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/app.config.ts +++ b/dev-packages/e2e-tests/test-applications/solidstart/app.config.ts @@ -15,7 +15,7 @@ export default defineConfig( }, { sentryUrl: 'http://localhost:3032', - authToken: 'fake-auth-token', + authToken: process.env.E2E_NO_UPLOAD ? undefined : 'fake-auth-token', org: 'test-org', project: 'test-project', release: { name: 'test-release' }, diff --git a/dev-packages/e2e-tests/test-applications/solidstart/assert-build.ts b/dev-packages/e2e-tests/test-applications/solidstart/assert-build.ts index fd40cfc4de2a..40b621444c7c 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/assert-build.ts +++ b/dev-packages/e2e-tests/test-applications/solidstart/assert-build.ts @@ -1,16 +1,27 @@ +import * as assert from 'node:assert/strict'; import * as path from 'node:path'; -import { assertSourceMapUpload } from '@sentry-internal/test-utils'; +import { assertSourceMapUpload, findSourceMapFiles, findSourceMappingUrlComments } from '@sentry-internal/test-utils'; const CLIENT_OUTPUT = path.join('.output', 'public'); const SERVER_OUTPUT = path.join('.output', 'server'); -// The app does not set `build.sourcemap`, so the SDK enables hidden maps and must delete them after upload. -// Vinxi writes the server output after the SDK deleted the maps, so `.output/server` keeps Nitro's own maps, -// which carry no mappings. They are not served, so only the client output must be free of maps. -assertSourceMapUpload({ - outputDirs: [CLIENT_OUTPUT, SERVER_OUTPUT], - strippedDirs: [CLIENT_OUTPUT], - markers: { 'client source': 'SOURCEMAP_MARKER_CLIENT', 'server source': 'SOURCEMAP_MARKER_SERVER' }, -}); +// The app does not set `build.sourcemap`, so the SDK enables hidden maps and asks the bundler plugin to +// remove them, also when no upload runs. Vinxi writes the server output after that, so `.output/server` +// keeps Nitro's own maps, which carry no mappings. They are not served, so only the client output must +// be free of maps. +if (process.env.E2E_NO_UPLOAD) { + assert.deepEqual(findSourceMapFiles({ outputDir: CLIENT_OUTPUT }), [], `Expected no source maps in ${CLIENT_OUTPUT}`); + assert.deepEqual( + findSourceMappingUrlComments({ outputDir: CLIENT_OUTPUT }), + [], + `Expected no sourceMappingURL comments in ${CLIENT_OUTPUT}`, + ); +} else { + assertSourceMapUpload({ + outputDirs: [CLIENT_OUTPUT, SERVER_OUTPUT], + strippedDirs: [CLIENT_OUTPUT], + markers: { 'client source': 'SOURCEMAP_MARKER_CLIENT', 'server source': 'SOURCEMAP_MARKER_SERVER' }, + }); +} console.log('All sourcemap assertions passed!'); diff --git a/dev-packages/e2e-tests/test-applications/solidstart/package.json b/dev-packages/e2e-tests/test-applications/solidstart/package.json index 58542cf48282..cf2fcd0e1533 100644 --- a/dev-packages/e2e-tests/test-applications/solidstart/package.json +++ b/dev-packages/e2e-tests/test-applications/solidstart/package.json @@ -11,6 +11,8 @@ "test:build": "pnpm install && pnpm build", "test:assert": "node assert-build.ts && pnpm test:prod && pnpm test:dev", "test:dev": "TEST_ENV=development playwright test db", + "test:build:no-upload": "pnpm install && E2E_NO_UPLOAD=1 pnpm build", + "test:assert:no-upload": "E2E_NO_UPLOAD=1 node assert-build.ts", "test:build:tunnel-basepath": "pnpm install && E2E_TEST_TUNNEL=1 E2E_TEST_BASE_PATH=/app pnpm build", "test:assert:tunnel-basepath": "E2E_TEST_TUNNEL=1 E2E_TEST_BASE_PATH=/app E2E_TEST_DSN=http://public@localhost:3031/1337 TEST_ENV=production playwright test tunnel", "test:assert:tunnel-basepath-dev": "E2E_TEST_TUNNEL=1 E2E_TEST_BASE_PATH=/app E2E_TEST_DSN=http://public@localhost:3031/1337 TEST_ENV=development playwright test tunnel" @@ -35,6 +37,11 @@ }, "sentryTest": { "variants": [ + { + "build-command": "pnpm test:build:no-upload", + "assert-command": "pnpm test:assert:no-upload", + "label": "solidstart (no upload)" + }, { "build-command": "pnpm test:build:tunnel-basepath", "assert-command": "pnpm test:assert:tunnel-basepath", diff --git a/packages/solidstart/src/vite/sentrySolidStart.ts b/packages/solidstart/src/vite/sentrySolidStart.ts index b06b10523564..7ee5758f101b 100644 --- a/packages/solidstart/src/vite/sentrySolidStart.ts +++ b/packages/solidstart/src/vite/sentrySolidStart.ts @@ -6,6 +6,7 @@ import { import { setupSentryNitroModule } from '@sentry/nitro'; import { sentryOrchestrionPlugin } from '@sentry/server-utils/orchestrion/vite'; import type { Plugin, UserConfig } from 'vite'; +import type { SourceMapState } from './sourceMaps'; import { makeAddSentryVitePluginSolidStart2, makeEnableSourceMapsVitePlugin } from './sourceMaps'; /** @@ -56,7 +57,14 @@ export function sentrySolidStart(options: SentrySolidStartOptions = {}): Plugin[ plugins.push(sentryOrchestrionPlugin({ buildTimeInstrumentation: options.buildTimeInstrumentation })); if (options.sourcemaps?.disable !== true) { - plugins.push(...makeAddSentryVitePluginSolidStart2(options), ...makeEnableSourceMapsVitePlugin(options)); + const sourceMapState: SourceMapState = { sdkEnabledSourceMaps: false }; + + // The bundler plugin decides on the `build.sourcemap` it finds in its own `config` hook, + // so the plugin that sets it has to come first. + plugins.push( + ...makeEnableSourceMapsVitePlugin(options, sourceMapState), + ...makeAddSentryVitePluginSolidStart2(options, sourceMapState), + ); } return plugins; diff --git a/packages/solidstart/src/vite/sentrySolidStartVite.ts b/packages/solidstart/src/vite/sentrySolidStartVite.ts index 61b884c29d2e..61a7175f7d4a 100644 --- a/packages/solidstart/src/vite/sentrySolidStartVite.ts +++ b/packages/solidstart/src/vite/sentrySolidStartVite.ts @@ -1,17 +1,14 @@ import { warnOnRemovedBuildOptions } from '@sentry/core'; import type { Plugin, UserConfig } from 'vite'; import { makeBuildInstrumentationFilePlugin } from './buildInstrumentationFile'; +import type { SourceMapState } from './sourceMaps'; import { makeAddSentryVitePlugin, makeEnableSourceMapsVitePlugin } from './sourceMaps'; import type { SentrySolidStartPluginOptions } from './types'; /** * Various Sentry vite plugins to be used for SolidStart. */ -export const sentrySolidStartVite = ( - options: SentrySolidStartPluginOptions = {}, - viteConfig: UserConfig, - tunnelBasePath = '', -): Plugin[] => { +export const sentrySolidStartVite = (options: SentrySolidStartPluginOptions = {}, tunnelBasePath = ''): Plugin[] => { warnOnRemovedBuildOptions(options, ['unstable_sentryVitePluginOptions']); const sentryPlugins: Plugin[] = [ @@ -30,10 +27,14 @@ export const sentrySolidStartVite = ( if (process.env.NODE_ENV !== 'development') { if (options.sourcemaps?.disable !== true) { - const sourceMapsPlugin = makeAddSentryVitePlugin(options, viteConfig); - const enableSourceMapsPlugin = makeEnableSourceMapsVitePlugin(options); - - sentryPlugins.push(...sourceMapsPlugin, ...enableSourceMapsPlugin); + const sourceMapState: SourceMapState = { sdkEnabledSourceMaps: false }; + + // The bundler plugin decides on the `build.sourcemap` it finds in its own `config` hook, + // so the plugin that sets it has to come first. + sentryPlugins.push( + ...makeEnableSourceMapsVitePlugin(options, sourceMapState), + ...makeAddSentryVitePlugin(options, sourceMapState), + ); } } @@ -49,7 +50,7 @@ export const addSentryPluginToVite = ( tunnelBasePath?: string, ): UserConfig => { const plugins = Array.isArray(config.plugins) ? [...config.plugins] : []; - plugins.unshift(sentrySolidStartVite(options, config, tunnelBasePath)); + plugins.unshift(sentrySolidStartVite(options, tunnelBasePath)); return { ...config, diff --git a/packages/solidstart/src/vite/sourceMaps.ts b/packages/solidstart/src/vite/sourceMaps.ts index 6e48ab647077..e0b68fe36558 100644 --- a/packages/solidstart/src/vite/sourceMaps.ts +++ b/packages/solidstart/src/vite/sourceMaps.ts @@ -6,12 +6,48 @@ import type { SentrySolidStartPluginOptions } from './types'; // `debug` is all the source map setting logic needs, and all the two majors' option types share. type SourceMapSettingOptions = { debug?: boolean }; -type FilesToDeleteAfterUpload = string | string[] | undefined; +/** Shared by the two plugins of one build. */ +export type SourceMapState = { + /** The SDK turned on hidden source maps because the user did not set `build.sourcemap`. */ + sdkEnabledSourceMaps: boolean; +}; + +type SourceMapsOptions = SentrySolidStartPluginOptions['sourcemaps']; + +/** + * The bundler plugin calls these when it decides, after `makeEnableSourceMapsVitePlugin` ran. + * It passes the name of an environment with its own `build.sourcemap`, and `undefined` for the + * top-level setting. The SDK only turns on the top-level maps: Nitro's environments get their + * setting from Nitro, and `@sentry/nitro` handles their maps. + * + * With `keepWhenUploadDisabled`, maps the SDK turned on stay with `disable: 'disable-upload'`. + */ +function makeSourceMapDeletionOptions( + sourcemaps: SourceMapsOptions, + state: SourceMapState, + { keepWhenUploadDisabled }: { keepWhenUploadDisabled: boolean }, +): { + deleteSourcemapsAfterUpload: (environment?: string) => boolean | undefined; + sdkEnabledSourceMaps: (environment?: string) => boolean; +} { + const sdkEnabledSourceMaps = (environment?: string): boolean => + environment === undefined && state.sdkEnabledSourceMaps; + + return { + deleteSourcemapsAfterUpload: environment => + sourcemaps?.deleteSourcemapsAfterUpload ?? + (sdkEnabledSourceMaps(environment) && !(keepWhenUploadDisabled && sourcemaps?.disable) ? true : undefined), + sdkEnabledSourceMaps, + }; +} /** * A Sentry plugin for adding the @sentry/bundler-plugins/vite plugin to automatically upload source maps to Sentry. */ -export function makeAddSentryVitePlugin(options: SentrySolidStartPluginOptions, viteConfig: UserConfig): Plugin[] { +export function makeAddSentryVitePlugin( + options: SentrySolidStartPluginOptions, + state: SourceMapState = { sdkEnabledSourceMaps: false }, +): Plugin[] { const { applicationKey, authToken, @@ -29,51 +65,30 @@ export function makeAddSentryVitePlugin(options: SentrySolidStartPluginOptions, telemetry, } = options; - let updatedFilesToDeleteAfterUpload: string[] | undefined = undefined; - - if ( - typeof sourcemaps?.filesToDeleteAfterUpload === 'undefined' && - // Only if source maps were previously not set, we update the "filesToDeleteAfterUpload" (as we override the setting with "hidden") - typeof viteConfig.build?.sourcemap === 'undefined' - ) { - // For .output, .vercel, .netlify etc. - updatedFilesToDeleteAfterUpload = ['.*/**/*.map']; - - debug && - // eslint-disable-next-line no-console - console.log( - `[Sentry] Automatically setting \`sourcemaps.filesToDeleteAfterUpload: ${JSON.stringify( - updatedFilesToDeleteAfterUpload, - )}\` to delete generated source maps after they were uploaded to Sentry.`, - ); - } - - return [ - ...sentryVitePlugin({ - applicationKey, - authToken: authToken ?? process.env.SENTRY_AUTH_TOKEN, - bundleSizeOptimizations, - debug: debug ?? false, - errorHandler, - headers, - moduleMetadata, - org: org ?? process.env.SENTRY_ORG, - project: project ?? process.env.SENTRY_PROJECT, - release, - silent, - telemetry: telemetry ?? true, - url: sentryUrl, - sourcemaps: { - ...sourcemaps, - filesToDeleteAfterUpload: sourcemaps?.filesToDeleteAfterUpload ?? updatedFilesToDeleteAfterUpload, - }, - _metaOptions: { - telemetry: { - metaFramework: 'solidstart', - }, + return sentryVitePlugin({ + applicationKey, + authToken: authToken ?? process.env.SENTRY_AUTH_TOKEN, + bundleSizeOptimizations, + debug: debug ?? false, + errorHandler, + headers, + moduleMetadata, + org: org ?? process.env.SENTRY_ORG, + project: project ?? process.env.SENTRY_PROJECT, + release, + silent, + telemetry: telemetry ?? true, + url: sentryUrl, + sourcemaps: { + ...sourcemaps, + ...makeSourceMapDeletionOptions(sourcemaps, state, { keepWhenUploadDisabled: false }), + }, + _metaOptions: { + telemetry: { + metaFramework: 'solidstart', }, - }), - ]; + }, + }); } /** @@ -83,7 +98,10 @@ export function makeAddSentryVitePlugin(options: SentrySolidStartPluginOptions, * Covers the Vite-built client assets only; Nitro emits the server bundle outside Vite's output dir * and the Sentry Nitro module uploads that. */ -export function makeAddSentryVitePluginSolidStart2(options: SentrySolidStartOptions): Plugin[] { +export function makeAddSentryVitePluginSolidStart2( + options: SentrySolidStartOptions, + state: SourceMapState = { sdkEnabledSourceMaps: false }, +): Plugin[] { // Everything not destructured out is field-for-field what `sentryVitePlugin` accepts, so it is // spread through — a new shared option then reaches the plugin without editing a list here. const { @@ -98,38 +116,7 @@ export function makeAddSentryVitePluginSolidStart2(options: SentrySolidStartOpti ...passthroughOptions } = options; - // Deferred because the default depends on whether the user set `build.sourcemap` themselves, - // which is only known once Vite resolves its config. - let resolveFilesToDeleteAfterUpload: - | ((value: FilesToDeleteAfterUpload | PromiseLike) => void) - | undefined; - const filesToDeleteAfterUploadPromise = new Promise(resolve => { - resolveFilesToDeleteAfterUpload = resolve; - }); - - const configPlugin: Plugin = { - name: 'sentry-solidstart-files-to-delete-after-upload', - apply: 'build', - enforce: 'post', - config(config) { - const userFilesToDelete = sourcemaps?.filesToDeleteAfterUpload; - - // Only clean up source maps we turned on ourselves. - if (typeof userFilesToDelete === 'undefined' && typeof config.build?.sourcemap === 'undefined') { - if (debug) { - // eslint-disable-next-line no-console - console.log( - '[Sentry] Automatically setting `sourcemaps.filesToDeleteAfterUpload: ["./**/*.map"]` to delete generated source maps after they were uploaded to Sentry.', - ); - } - resolveFilesToDeleteAfterUpload?.(['./**/*.map']); - } else { - resolveFilesToDeleteAfterUpload?.(userFilesToDelete); - } - }, - }; - - const sentryPlugins = sentryVitePlugin({ + return sentryVitePlugin({ ...passthroughOptions, authToken: authToken ?? process.env.SENTRY_AUTH_TOKEN, debug: debug ?? false, @@ -139,7 +126,8 @@ export function makeAddSentryVitePluginSolidStart2(options: SentrySolidStartOpti url: sentryUrl, sourcemaps: { ...sourcemaps, - filesToDeleteAfterUpload: filesToDeleteAfterUploadPromise, + // `@sentry/nitro` keeps the server maps with `disable-upload`, so the client maps stay too. + ...makeSourceMapDeletionOptions(sourcemaps, state, { keepWhenUploadDisabled: true }), }, _metaOptions: { telemetry: { @@ -147,27 +135,29 @@ export function makeAddSentryVitePluginSolidStart2(options: SentrySolidStartOpti }, }, }); - - return [configPlugin, ...sentryPlugins]; } /** * A Sentry plugin for SolidStart to enable "hidden" source maps if they are unset. Used by both * SolidStart majors. */ -export function makeEnableSourceMapsVitePlugin(options: SourceMapSettingOptions): Plugin[] { +export function makeEnableSourceMapsVitePlugin( + options: SourceMapSettingOptions, + state: SourceMapState = { sdkEnabledSourceMaps: false }, +): Plugin[] { return [ { name: 'sentry-solidstart-update-source-map-setting', apply: 'build', enforce: 'post', config(viteConfig) { + const sourcemap = getUpdatedSourceMapSettings(viteConfig, options); + state.sdkEnabledSourceMaps = sourcemap !== viteConfig.build?.sourcemap; + // Return only what changed: Vite concatenates arrays when merging a `config` return value, // so echoing the whole config back would duplicate every array the user had. return { - build: { - sourcemap: getUpdatedSourceMapSettings(viteConfig, options), - }, + build: { sourcemap }, }; }, }, @@ -185,7 +175,7 @@ export function makeEnableSourceMapsVitePlugin(options: SourceMapSettingOptions) * * 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) + * - pass `deleteSourcemapsAfterUpload: true`, so the maps never reach the build output * * --> only exported for testing */ diff --git a/packages/solidstart/test/config/withSentry.test.ts b/packages/solidstart/test/config/withSentry.test.ts index d4074ce9cea5..0d779f145d72 100644 --- a/packages/solidstart/test/config/withSentry.test.ts +++ b/packages/solidstart/test/config/withSentry.test.ts @@ -128,9 +128,9 @@ describe('withSentry()', () => { expect(names).toEqual([ 'sentry-solidstart-tunnel-base-path', 'sentry-solidstart-build-instrumentation-file', + 'sentry-solidstart-update-source-map-setting', 'sentry-vite-plugin', 'sentry-vite-upload-staging', - 'sentry-solidstart-update-source-map-setting', ]); }); @@ -153,9 +153,9 @@ describe('withSentry()', () => { expect(names).toEqual([ 'sentry-solidstart-tunnel-base-path', 'sentry-solidstart-build-instrumentation-file', + 'sentry-solidstart-update-source-map-setting', 'sentry-vite-plugin', 'sentry-vite-upload-staging', - 'sentry-solidstart-update-source-map-setting', 'my-test-plugin', ]); }); @@ -182,9 +182,9 @@ describe('withSentry()', () => { expect(names).toEqual([ 'sentry-solidstart-tunnel-base-path', 'sentry-solidstart-build-instrumentation-file', + 'sentry-solidstart-update-source-map-setting', 'sentry-vite-plugin', 'sentry-vite-upload-staging', - 'sentry-solidstart-update-source-map-setting', 'my-test-plugin', ]); }); diff --git a/packages/solidstart/test/vite/sentrySolidStart.test.ts b/packages/solidstart/test/vite/sentrySolidStart.test.ts index 2228da8c5fae..01d7466883cb 100644 --- a/packages/solidstart/test/vite/sentrySolidStart.test.ts +++ b/packages/solidstart/test/vite/sentrySolidStart.test.ts @@ -53,10 +53,9 @@ describe('sentrySolidStart()', () => { 'sentry-solidstart-server-entry', // `sentryOrchestrionPlugin` spreads the upstream code-transformer plugin, which brings its name. 'code-transformer', - 'sentry-solidstart-files-to-delete-after-upload', + 'sentry-solidstart-update-source-map-setting', 'sentry-vite-plugin', 'sentry-vite-upload-staging', - 'sentry-solidstart-update-source-map-setting', ]); }); diff --git a/packages/solidstart/test/vite/sentrySolidStartVite.test.ts b/packages/solidstart/test/vite/sentrySolidStartVite.test.ts index 3273693769cf..6d1c76c851f0 100644 --- a/packages/solidstart/test/vite/sentrySolidStartVite.test.ts +++ b/packages/solidstart/test/vite/sentrySolidStartVite.test.ts @@ -10,15 +10,12 @@ vi.spyOn(console, 'warn').mockImplementation(() => { }); function getSentrySolidStartVitePlugins(options?: Parameters[0]): Plugin[] { - return sentrySolidStartVite( - { - project: 'project', - org: 'org', - authToken: 'token', - ...options, - }, - {}, - ); + return sentrySolidStartVite({ + project: 'project', + org: 'org', + authToken: 'token', + ...options, + }); } describe('sentrySolidStartVite()', () => { @@ -28,9 +25,9 @@ describe('sentrySolidStartVite()', () => { expect(names).toEqual([ 'sentry-solidstart-tunnel-base-path', 'sentry-solidstart-build-instrumentation-file', + 'sentry-solidstart-update-source-map-setting', 'sentry-vite-plugin', 'sentry-vite-upload-staging', - 'sentry-solidstart-update-source-map-setting', ]); }); diff --git a/packages/solidstart/test/vite/sourceMaps.test.ts b/packages/solidstart/test/vite/sourceMaps.test.ts index d1d82b4866e7..91d9f2c03827 100644 --- a/packages/solidstart/test/vite/sourceMaps.test.ts +++ b/packages/solidstart/test/vite/sourceMaps.test.ts @@ -1,7 +1,9 @@ -import type { SentryVitePluginOptions } from '@sentry/bundler-plugins/vite'; +import type { SentryVitePluginInternalOptions } from '@sentry/bundler-plugins/vite'; import type { Plugin, UserConfig } from 'vite'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { sentrySolidStart } from '../../src/vite/sentrySolidStart'; +import { sentrySolidStartVite } from '../../src/vite/sentrySolidStartVite'; +import type { SourceMapState } from '../../src/vite/sourceMaps'; import { getUpdatedSourceMapSettings, makeAddSentryVitePlugin, @@ -9,25 +11,46 @@ import { makeEnableSourceMapsVitePlugin, } from '../../src/vite/sourceMaps'; -const mockedSentryVitePlugin = { - name: 'sentry-vite-debug-id-upload-plugin', - writeBundle: vi.fn(), -}; +let lastPluginOptions: SentryVitePluginInternalOptions | undefined; -// Captured so tests can await the deferred `filesToDeleteAfterUpload` promise, which -// `toHaveBeenCalledWith` can only match by identity. -let lastPluginOptions: SentryVitePluginOptions | undefined; +/** What the options returned when the mocked Sentry plugin's `config` hook ran. */ +let optionsAtSentryConfigHook: { deletion?: boolean; sdkEnabled?: boolean } | undefined; -const sentryVitePluginSpy = vi.fn((options: SentryVitePluginOptions) => { +const sentryVitePluginSpy = vi.fn((options: SentryVitePluginInternalOptions): Plugin[] => { lastPluginOptions = options; - return [mockedSentryVitePlugin]; + return [ + { + name: 'sentry-vite-debug-id-upload-plugin', + config() { + optionsAtSentryConfigHook = { deletion: getDeleteOption(), sdkEnabled: getSdkEnabledOption() }; + }, + }, + ]; }); -/** Runs a plugin's `config` hook the way Vite does. */ -function runConfigHook(plugin: Plugin, config: UserConfig = {}): void { - const hook = plugin.config; - const handler = typeof hook === 'function' ? hook : hook?.handler; - handler?.call({} as never, config, { command: 'build', mode: 'production' }); +/** Runs each plugin's `config` hook in order over one config, merging `build` the way Vite does. */ +function runConfigHooks(plugins: Plugin[], config: UserConfig): UserConfig { + for (const plugin of plugins) { + const hook = plugin.config; + const handler = typeof hook === 'function' ? hook : hook?.handler; + const contributed = handler?.call({} as never, config, { command: 'build', mode: 'production' }) as + | UserConfig + | undefined; + if (contributed?.build) { + config.build = { ...config.build, ...contributed.build }; + } + } + return config; +} + +function getDeleteOption(environment?: string): boolean | undefined { + const value = lastPluginOptions?.sourcemaps?.deleteSourcemapsAfterUpload; + return typeof value === 'function' ? value(environment) : value; +} + +function getSdkEnabledOption(environment?: string): boolean | undefined { + const value = lastPluginOptions?.sourcemaps?.sdkEnabledSourceMaps; + return typeof value === 'function' ? value(environment) : value; } vi.mock('@sentry/bundler-plugins/vite', async () => { @@ -35,12 +58,13 @@ vi.mock('@sentry/bundler-plugins/vite', async () => { return { ...original, - sentryVitePlugin: (options: SentryVitePluginOptions) => sentryVitePluginSpy(options), + sentryVitePlugin: (options: SentryVitePluginInternalOptions) => sentryVitePluginSpy(options), }; }); beforeEach(() => { vi.clearAllMocks(); + optionsAtSentryConfigHook = undefined; }); describe('makeSourceMapsVitePlugin()', () => { @@ -73,26 +97,44 @@ describe('makeSourceMapsVitePlugin()', () => { expect(contributed).toEqual({ build: { sourcemap: 'hidden' } }); }); + + it('records that it enabled source maps', () => { + const state: SourceMapState = { sdkEnabledSourceMaps: false }; + + runConfigHooks(makeEnableSourceMapsVitePlugin({}, state), {}); + + expect(state.sdkEnabledSourceMaps).toBe(true); + }); + + it.each([['hidden'], ['inline'], [true], [false]] as const)( + 'does not record a `%s` setting the user made', + setting => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + const state: SourceMapState = { sdkEnabledSourceMaps: true }; + + runConfigHooks(makeEnableSourceMapsVitePlugin({}, state), { build: { sourcemap: setting } }); + + expect(state.sdkEnabledSourceMaps).toBe(false); + }, + ); }); describe('makeAddSentryVitePlugin()', () => { it('passes user-specified vite plugin options to vite plugin', () => { - makeAddSentryVitePlugin( - { - org: 'my-org', - authToken: 'my-token', - applicationKey: 'my-app-key', - sentryUrl: 'https://my.sentry.io', - moduleMetadata: { team: 'sdk' }, - sourcemaps: { - filesToDeleteAfterUpload: ['baz/*.js'], - }, - bundleSizeOptimizations: { - excludeTracing: true, - }, + makeAddSentryVitePlugin({ + org: 'my-org', + authToken: 'my-token', + applicationKey: 'my-app-key', + sentryUrl: 'https://my.sentry.io', + moduleMetadata: { team: 'sdk' }, + sourcemaps: { + filesToDeleteAfterUpload: ['baz/*.js'], }, - {}, - ); + bundleSizeOptimizations: { + excludeTracing: true, + }, + }); expect(sentryVitePluginSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -104,6 +146,8 @@ describe('makeAddSentryVitePlugin()', () => { moduleMetadata: { team: 'sdk' }, sourcemaps: { filesToDeleteAfterUpload: ['baz/*.js'], + deleteSourcemapsAfterUpload: expect.any(Function), + sdkEnabledSourceMaps: expect.any(Function), }, bundleSizeOptimizations: { excludeTracing: true, @@ -111,69 +155,6 @@ describe('makeAddSentryVitePlugin()', () => { }), ); }); - - it('should update `filesToDeleteAfterUpload` if source map generation was previously not defined', () => { - makeAddSentryVitePlugin( - { - org: 'my-org', - authToken: 'my-token', - bundleSizeOptimizations: { - excludeTracing: true, - }, - }, - {}, - ); - - expect(sentryVitePluginSpy).toHaveBeenCalledWith( - expect.objectContaining({ - sourcemaps: expect.objectContaining({ - filesToDeleteAfterUpload: ['.*/**/*.map'], - }), - }), - ); - }); - - it('should not update `filesToDeleteAfterUpload` if source map generation was previously enabled', () => { - makeAddSentryVitePlugin( - { - org: 'my-org', - authToken: 'my-token', - bundleSizeOptimizations: { - excludeTracing: true, - }, - }, - { build: { sourcemap: true } }, - ); - - expect(sentryVitePluginSpy).toHaveBeenCalledWith( - expect.objectContaining({ - sourcemaps: expect.objectContaining({ - filesToDeleteAfterUpload: undefined, - }), - }), - ); - }); - - it('should not update `filesToDeleteAfterUpload` if source map generation was previously disabled', () => { - makeAddSentryVitePlugin( - { - org: 'my-org', - authToken: 'my-token', - bundleSizeOptimizations: { - excludeTracing: true, - }, - }, - { build: { sourcemap: false } }, - ); - - expect(sentryVitePluginSpy).toHaveBeenCalledWith( - expect.objectContaining({ - sourcemaps: expect.objectContaining({ - filesToDeleteAfterUpload: undefined, - }), - }), - ); - }); }); describe('makeAddSentryVitePluginSolidStart2()', () => { @@ -221,44 +202,116 @@ describe('makeAddSentryVitePluginSolidStart2()', () => { ); }); - // `filesToDeleteAfterUpload` is resolved through a deferred promise, because the default depends - // on whether the user set `build.sourcemap` themselves - only known once Vite resolves its config. - it('resolves `filesToDeleteAfterUpload` to the user-specified value', async () => { - const plugins = makeAddSentryVitePluginSolidStart2({ - sourcemaps: { filesToDeleteAfterUpload: ['custom/**/*.map'] }, - }); + it('warns when the removed `unstable_sentryVitePluginOptions` is still set', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + // @ts-expect-error - removed in v11, but JS configs get no type checking + sentrySolidStart({ unstable_sentryVitePluginOptions: { org: 'other-org' } }); + + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('unstable_sentryVitePluginOptions')); + + consoleWarnSpy.mockRestore(); + }); +}); + +describe.each([ + ['makeAddSentryVitePlugin', makeAddSentryVitePlugin], + ['makeAddSentryVitePluginSolidStart2', makeAddSentryVitePluginSolidStart2], +] as const)('%s() source map deletion', (_name, makePlugin) => { + it('asks for deletion once the SDK enabled the source maps', () => { + const state: SourceMapState = { sdkEnabledSourceMaps: false }; + makePlugin({}, state); + + expect(getDeleteOption()).toBeUndefined(); + state.sdkEnabledSourceMaps = true; + expect(getDeleteOption()).toBe(true); + }); - runConfigHook(plugins[0]!); + it.each([true, false])('passes a user-set `deleteSourcemapsAfterUpload: %s` through', value => { + makePlugin({ sourcemaps: { deleteSourcemapsAfterUpload: value } }, { sdkEnabledSourceMaps: !value }); - await expect(lastPluginOptions?.sourcemaps?.filesToDeleteAfterUpload).resolves.toEqual(['custom/**/*.map']); + expect(getDeleteOption()).toBe(value); }); - // Only clean up source maps the SDK turned on itself. - it('defaults `filesToDeleteAfterUpload` only when the user left `build.sourcemap` unset', async () => { - const plugins = makeAddSentryVitePluginSolidStart2({ org: 'my-org' }); + it('asks for deletion next to a user-set `filesToDeleteAfterUpload`', () => { + makePlugin({ sourcemaps: { filesToDeleteAfterUpload: ['./dist/**/*.map'] } }, { sdkEnabledSourceMaps: true }); - runConfigHook(plugins[0]!); + expect(lastPluginOptions?.sourcemaps?.filesToDeleteAfterUpload).toEqual(['./dist/**/*.map']); + expect(getDeleteOption()).toBe(true); + }); - await expect(lastPluginOptions?.sourcemaps?.filesToDeleteAfterUpload).resolves.toEqual(['./**/*.map']); + // Nitro gives its environments their own `build.sourcemap`, and `@sentry/nitro` reads those + // maps after the build. + it.each([ + [undefined, true, true], + ['nitro', undefined, false], + ['ssr', undefined, false], + ])('answers for the `%s` environment', (environment, deletion, sdkEnabled) => { + makePlugin({}, { sdkEnabledSourceMaps: true }); + + expect(getDeleteOption(environment)).toBe(deletion); + expect(getSdkEnabledOption(environment)).toBe(sdkEnabled); }); - it('leaves `filesToDeleteAfterUpload` unset when the user configured `build.sourcemap`', async () => { - const plugins = makeAddSentryVitePluginSolidStart2({ org: 'my-org' }); + it.each([undefined, 'nitro', 'ssr'])( + 'passes a user-set `deleteSourcemapsAfterUpload` to the `%s` environment', + environment => { + makePlugin({ sourcemaps: { deleteSourcemapsAfterUpload: true } }, { sdkEnabledSourceMaps: true }); + + expect(getDeleteOption(environment)).toBe(true); + }, + ); - runConfigHook(plugins[0]!, { build: { sourcemap: true } }); + // Makes the plugin remove all of the maps, also those a user's `assets` or `ignore` leave out. + it('tells the bundler plugin whether the SDK enabled the source maps', () => { + const state: SourceMapState = { sdkEnabledSourceMaps: false }; + makePlugin({ sourcemaps: { assets: ['./dist/client/**'] } }, state); - await expect(lastPluginOptions?.sourcemaps?.filesToDeleteAfterUpload).resolves.toBeUndefined(); + expect(getSdkEnabledOption()).toBe(false); + state.sdkEnabledSourceMaps = true; + expect(getSdkEnabledOption()).toBe(true); + expect(getDeleteOption()).toBe(true); }); +}); - it('warns when the removed `unstable_sentryVitePluginOptions` is still set', () => { - const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); +it('makeAddSentryVitePlugin() asks for deletion when only the upload is disabled', () => { + makeAddSentryVitePlugin({ sourcemaps: { disable: 'disable-upload' } }, { sdkEnabledSourceMaps: true }); - // @ts-expect-error - removed in v11, but JS configs get no type checking - sentrySolidStart({ unstable_sentryVitePluginOptions: { org: 'other-org' } }); + expect(getDeleteOption()).toBe(true); + expect(getSdkEnabledOption()).toBe(true); +}); - expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('unstable_sentryVitePluginOptions')); +it('makeAddSentryVitePluginSolidStart2() keeps the source maps when only the upload is disabled', () => { + makeAddSentryVitePluginSolidStart2({ sourcemaps: { disable: 'disable-upload' } }, { sdkEnabledSourceMaps: true }); - consoleWarnSpy.mockRestore(); + expect(getDeleteOption()).toBeUndefined(); +}); + +// Vinxi passes its router to every SolidStart 1 `config` hook. +const vinxiClientConfig = { router: { target: 'client', name: 'client', root: '.' } } as UserConfig; + +// The bundler plugin reads the option when its own `config` hook runs, which comes after ours. +describe.each([ + ['sentrySolidStart', () => sentrySolidStart()], + ['sentrySolidStartVite', () => sentrySolidStartVite()], +] as const)('%s() in plugin order', (_name, getPlugins) => { + beforeEach(() => { + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + + it('enables hidden source maps and asks the bundler plugin to delete them', () => { + const config = runConfigHooks(getPlugins(), { ...vinxiClientConfig }); + + expect(config.build?.sourcemap).toBe('hidden'); + expect(optionsAtSentryConfigHook).toEqual({ deletion: true, sdkEnabled: true }); + }); + + it('keeps source maps the user enabled', () => { + const config = runConfigHooks(getPlugins(), { ...vinxiClientConfig, build: { sourcemap: 'hidden' } }); + + expect(config.build?.sourcemap).toBe('hidden'); + expect(optionsAtSentryConfigHook).toEqual({ deletion: undefined, sdkEnabled: false }); }); });