Skip to content
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.

Sign up for GitHub

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

fix(sveltekit): Avoid loading vite config to determine source maps setting #15440

Merged
merged 4 commits into from
Feb 20, 2025
Merged

fix(sveltekit): Avoid loading vite config to determine source maps setting #15440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85a8e11
fix(sveltekit): Avoid loading vite config to determine source maps se…
Lms24 Feb 18, 2025
28449b3
bump plugin to 3.2.0
Lms24 Feb 19, 2025
efaf8c5
adjust comments
Lms24 Feb 19, 2025
543d920
revert unrelated change
Lms24 Feb 19, 2025
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
6 changes: 2 additions & 4 deletions packages/sveltekit/package.json
View file
Open in desktop
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
Open in desktop
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);
Comment on lines +103 to +104
Copy link
Member Author

@Lms24 Lms24 Feb 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this is the ugly part of this PR: We always need to resolve the promise we pass to the vite plugin. So everywhere where we early return, in addition to the expected places.


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
Open in desktop
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
Loading