Skip to content

Commit

Permalink
fix(sveltekit): Avoid loading vite config to determine source maps se…
Browse files Browse the repository at this point in the history 
…tting (#15440)

- read sourcemap generation config in `config` hook of our source maps
settings sub plugin
- resolve the `filesToDeleteAfterUpload` promise whenever we know what
to set it to (using a promise allows us to defer this decision to plugin
hook runtime rather than plugin creation time)
- adusted tests
  • Loading branch information
@Lms24
Lms24 authored Feb 20, 2025
1 parent a0be1a5 commit 6c69710
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 144 deletions.
6 changes: 2 additions & 4 deletions packages/sveltekit/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
"engines": {
"node": ">=18"
},
"files": [
"/build"
],
"files": ["/build"],
"main": "build/cjs/index.server.js",
"module": "build/esm/index.server.js",
"browser": "build/esm/index.client.js",
Expand Down Expand Up @@ -44,7 +42,7 @@
"@sentry/node": "9.1.0",
"@sentry/opentelemetry": "9.1.0",
"@sentry/svelte": "9.1.0",
"@sentry/vite-plugin": "2.22.6",
"@sentry/vite-plugin": "3.2.0",
"magic-string": "0.30.7",
"magicast": "0.2.8",
"sorcery": "1.0.0"
Expand Down
174 changes: 85 additions & 89 deletions packages/sveltekit/src/vite/sourceMaps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@ type Sorcery = {
load(filepath: string): Promise<Chain>;
};

type GlobalWithSourceMapSetting = typeof globalThis & {
_sentry_sourceMapSetting?: {
updatedSourceMapSetting?: boolean | 'inline' | 'hidden';
previousSourceMapSetting?: UserSourceMapSetting;
};
};

// storing this in the module scope because `makeCustomSentryVitePlugin` is called multiple times
// and we only want to generate a uuid once in case we have to fall back to it.
const releaseName = detectSentryRelease();
Expand All @@ -57,8 +50,6 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
const usedAdapter = options?.adapter || 'other';
const adapterOutputDir = await getAdapterOutputDir(svelteConfig, usedAdapter);

const globalWithSourceMapSetting = globalThis as GlobalWithSourceMapSetting;

const defaultPluginOptions: SentryVitePluginOptions = {
release: {
name: releaseName,
Expand All @@ -70,61 +61,8 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
},
};

// Including all hidden (`.*`) directories by default so that folders like .vercel,
// .netlify, etc are also cleaned up. Additionally, we include the adapter output
// dir which could be a non-hidden directory, like `build` for the Node adapter.
const defaultFileDeletionGlob = ['./.*/**/*.map', `./${adapterOutputDir}/**/*.map`];

if (!globalWithSourceMapSetting._sentry_sourceMapSetting) {
let configFile: {
path: string;
config: UserConfig;
dependencies: string[];
} | null = null;

try {
// @ts-expect-error - the dynamic import here works fine
const Vite = await import('vite');
configFile = await Vite.loadConfigFromFile({ command: 'build', mode: 'production' });
} catch {
if (options?.debug) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
'[Sentry] Could not import Vite to load your vite config. Please set `build.sourcemap` to `true` or `hidden` to enable source map generation.',
);
});
}
}

if (configFile) {
globalWithSourceMapSetting._sentry_sourceMapSetting = getUpdatedSourceMapSetting(configFile.config);
} else {
if (options?.debug) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
'[Sentry] Could not load Vite config with Vite "production" mode. This is needed for Sentry to automatically update source map settings.',
);
});
}
}

if (options?.debug && globalWithSourceMapSetting._sentry_sourceMapSetting?.previousSourceMapSetting === 'unset') {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
`[Sentry] Automatically setting \`sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload: [${defaultFileDeletionGlob
.map(file => `"${file}"`)
.join(', ')}]\` to delete generated source maps after they were uploaded to Sentry.`,
);
});
}
}

const shouldDeleteDefaultSourceMaps =
globalWithSourceMapSetting._sentry_sourceMapSetting?.previousSourceMapSetting === 'unset' &&
!options?.sourcemaps?.filesToDeleteAfterUpload;
const { promise: filesToDeleteAfterUpload, resolve: resolveFilesToDeleteAfterUpload } =
createFilesToDeleteAfterUploadPromise();

const mergedOptions = {
...defaultPluginOptions,
Expand All @@ -135,9 +73,7 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
},
sourcemaps: {
...options?.sourcemaps,
filesToDeleteAfterUpload: shouldDeleteDefaultSourceMaps
? defaultFileDeletionGlob
: options?.sourcemaps?.filesToDeleteAfterUpload,
filesToDeleteAfterUpload,
},
};

Expand All @@ -163,6 +99,10 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
console.warn(
'sentry-vite-debug-id-upload-plugin not found in sentryPlugins! Cannot modify plugin - returning default Sentry Vite plugins',
);

// resolving filesToDeleteAfterUpload here, because we return the original deletion plugin which awaits the promise
resolveFilesToDeleteAfterUpload(undefined);

return sentryPlugins;
}

Expand All @@ -172,6 +112,10 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
console.warn(
'sentry-file-deletion-plugin not found in sentryPlugins! Cannot modify plugin - returning default Sentry Vite plugins',
);

// resolving filesToDeleteAfterUpload here, because we return the original deletion plugin which awaits the promise
resolveFilesToDeleteAfterUpload(undefined);

return sentryPlugins;
}

Expand All @@ -181,6 +125,10 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
console.warn(
'sentry-release-management-plugin not found in sentryPlugins! Cannot modify plugin - returning default Sentry Vite plugins',
);

// resolving filesToDeleteAfterUpload here, because we return the original deletion plugin which awaits the promise
resolveFilesToDeleteAfterUpload(undefined);

return sentryPlugins;
}

Expand All @@ -205,37 +153,66 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
const sourceMapSettingsPlugin: Plugin = {
name: 'sentry-sveltekit-update-source-map-setting-plugin',
apply: 'build', // only apply this plugin at build time
config: (config: UserConfig) => {
config: async (config: UserConfig) => {
const settingKey = 'build.sourcemap';

if (globalWithSourceMapSetting._sentry_sourceMapSetting?.previousSourceMapSetting === 'unset') {
const { updatedSourceMapSetting, previousSourceMapSetting } = getUpdatedSourceMapSetting(config);

const userProvidedFilesToDeleteAfterUpload = await options?.sourcemaps?.filesToDeleteAfterUpload;

if (previousSourceMapSetting === 'unset') {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.log(`[Sentry] Enabled source map generation in the build options with \`${settingKey}: "hidden"\`.`);
});

if (userProvidedFilesToDeleteAfterUpload) {
resolveFilesToDeleteAfterUpload(userProvidedFilesToDeleteAfterUpload);
} else {
// Including all hidden (`.*`) directories by default so that folders like .vercel,
// .netlify, etc are also cleaned up. Additionally, we include the adapter output
// dir which could be a non-hidden directory, like `build` for the Node adapter.
const defaultFileDeletionGlob = ['./.*/**/*.map', `./${adapterOutputDir}/**/*.map`];

consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
`[Sentry] Automatically setting \`sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload: [${defaultFileDeletionGlob
.map(file => `"${file}"`)
.join(', ')}]\` to delete generated source maps after they were uploaded to Sentry.`,
);
});

// In case we enabled source maps and users didn't specify a glob patter to delete, we set a default pattern:
resolveFilesToDeleteAfterUpload(defaultFileDeletionGlob);
}

return {
...config,
build: { ...config.build, sourcemap: 'hidden' },
build: { ...config.build, sourcemap: updatedSourceMapSetting },
};
} else if (globalWithSourceMapSetting._sentry_sourceMapSetting?.previousSourceMapSetting === 'disabled') {
}

if (previousSourceMapSetting === 'disabled') {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
`[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 (globalWithSourceMapSetting._sentry_sourceMapSetting?.previousSourceMapSetting === 'enabled') {
} else if (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.`,
`[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.`,
);
});
}
}

resolveFilesToDeleteAfterUpload(userProvidedFilesToDeleteAfterUpload);

return config;
},
};
Expand Down Expand Up @@ -423,7 +400,7 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug
/**
* Whether the user enabled (true, 'hidden', 'inline') or disabled (false) source maps
*/
export type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined;
type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined;

/** There are 3 ways to set up source map generation (https://github.com/getsentry/sentry-javascript/issues/13993)
*
Expand All @@ -445,25 +422,25 @@ export function getUpdatedSourceMapSetting(viteConfig: {
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';
const originalSourcemapSetting = viteConfig.build.sourcemap;

if (originalSourcemapSetting === false) {
return {
previousSourceMapSetting: 'disabled',
updatedSourceMapSetting: originalSourcemapSetting,
};
}

if (originalSourcemapSetting && ['hidden', 'inline', true].includes(originalSourcemapSetting)) {
return { previousSourceMapSetting: 'enabled', updatedSourceMapSetting: originalSourcemapSetting };
}

return { previousSourceMapSetting, updatedSourceMapSetting };
return {
previousSourceMapSetting: 'unset',
updatedSourceMapSetting: 'hidden',
};
}

function getFiles(dir: string): string[] {
Expand Down Expand Up @@ -499,3 +476,22 @@ function detectSentryRelease(): string {

return release;
}

/**
* Creates a deferred promise that can be resolved/rejected by calling the
* `resolve` or `reject` function.
* Inspired by: https://stackoverflow.com/a/69027809
*/
function createFilesToDeleteAfterUploadPromise(): {
promise: Promise<string | string[] | undefined>;
resolve: (value: string | string[] | undefined) => void;
reject: (reason?: unknown) => void;
} {
let resolve!: (value: string | string[] | undefined) => void;
let reject!: (reason?: unknown) => void;
const promise = new Promise<string | string[] | undefined>((res, rej) => {
resolve = res;
reject = rej;
});
return { resolve, reject, promise };
}
33 changes: 17 additions & 16 deletions packages/sveltekit/test/vite/sourceMaps.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { makeCustomSentryVitePlugins } from '../../src/vite/sourceMaps';
import { getUpdatedSourceMapSetting, makeCustomSentryVitePlugins } from '../../src/vite/sourceMaps';

import type { Plugin } from 'vite';

Expand Down Expand Up @@ -113,7 +113,7 @@ describe('makeCustomSentryVitePlugins()', () => {
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin');

// @ts-expect-error this function exists!
const sentryConfig = plugin.config(originalConfig);
const sentryConfig = await plugin.config(originalConfig);

expect(sentryConfig).toEqual(originalConfig);
});
Expand All @@ -132,7 +132,7 @@ describe('makeCustomSentryVitePlugins()', () => {
const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin');

// @ts-expect-error this function exists!
const sentryConfig = plugin.config(originalConfig);
const sentryConfig = await plugin.config(originalConfig);

expect(sentryConfig).toEqual({
build: {
Expand All @@ -155,7 +155,7 @@ describe('makeCustomSentryVitePlugins()', () => {

const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin');
// @ts-expect-error this function exists!
const sentryConfig = plugin.config(originalConfig);
const sentryConfig = await plugin.config(originalConfig);
expect(sentryConfig).toEqual({
...originalConfig,
build: {
Expand Down Expand Up @@ -320,22 +320,23 @@ describe('makeCustomSentryVitePlugins()', () => {
describe('changeViteSourceMapSettings()', () => {
const cases = [
{ sourcemap: false, expectedSourcemap: false, expectedPrevious: 'disabled' },
{ sourcemap: 'hidden', expectedSourcemap: 'hidden', expectedPrevious: 'enabled' },
{ sourcemap: 'inline', expectedSourcemap: 'inline', expectedPrevious: 'enabled' },
{ sourcemap: 'hidden' as const, expectedSourcemap: 'hidden', expectedPrevious: 'enabled' },
{ sourcemap: 'inline' as const, expectedSourcemap: 'inline', expectedPrevious: 'enabled' },
{ sourcemap: true, expectedSourcemap: true, expectedPrevious: 'enabled' },
{ sourcemap: undefined, expectedSourcemap: 'hidden', expectedPrevious: 'unset' },
];

it.each(cases)('handles vite source map settings $1', async ({ sourcemap, expectedSourcemap, expectedPrevious }) => {
const viteConfig = { build: { sourcemap } };
it.each(cases)(
'handles vite source map setting `build.sourcemap: $sourcemap`',
async ({ sourcemap, expectedSourcemap, expectedPrevious }) => {
const viteConfig = { build: { sourcemap } };

const { getUpdatedSourceMapSetting } = await import('../../src/vite/sourceMaps');
const result = getUpdatedSourceMapSetting(viteConfig);

const result = getUpdatedSourceMapSetting(viteConfig);

expect(result).toEqual({
updatedSourceMapSetting: expectedSourcemap,
previousSourceMapSetting: expectedPrevious,
});
});
expect(result).toEqual({
updatedSourceMapSetting: expectedSourcemap,
previousSourceMapSetting: expectedPrevious,
});
},
);
});
Loading

0 comments on commit 6c69710

Please sign in to comment.