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
28 changes: 12 additions & 16 deletions packages/astro/src/integration/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { sentryVitePlugin } from '@sentry/bundler-plugins/vite';
import { warnOnRemovedBuildOptions } from '@sentry/core';
import { sentryOrchestrionPlugin } from '@sentry/server-utils/orchestrion/vite';
import type { AstroConfig, AstroIntegration, AstroIntegrationLogger } from 'astro';
import * as fs from 'fs';
Expand Down Expand Up @@ -38,7 +39,7 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
buildTimeInstrumentation,
bundleSizeOptimizations,
applicationKey,
unstable_sentryVitePluginOptions,
moduleMetadata,
debug,
org,
project,
Expand All @@ -50,20 +51,20 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
errorHandler,
} = options;

warnOnRemovedBuildOptions(options, ['unstable_sentryVitePluginOptions'], message => logger.warn(message));
Comment thread
chargome marked this conversation as resolved.
// The nested spelling is not covered by the check above.
// eslint-disable-next-line typescript/no-deprecated
warnOnRemovedBuildOptions(options.sourceMapsUploadOptions, ['unstable_sentryVitePluginOptions'], message =>
logger.warn(message),
);

const sdkEnabled = {
client: typeof enabled === 'boolean' ? enabled : (enabled?.client ?? true),
server: typeof enabled === 'boolean' ? enabled : (enabled?.server ?? true),
};

const sourceMapsNeeded = sdkEnabled.client || sdkEnabled.server;
// eslint-disable-next-line typescript/no-deprecated
const { unstable_sentryVitePluginOptions: deprecatedVitePluginOptions, ...uploadOptions } =
sourceMapsUploadOptions || {};

const unstableMerged_sentryVitePluginOptions = {
...deprecatedVitePluginOptions,
...unstable_sentryVitePluginOptions,
};
const uploadOptions = sourceMapsUploadOptions || {};

const shouldUploadSourcemaps =
(sourceMapsNeeded &&
Expand Down Expand Up @@ -103,6 +104,7 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
plugins: [
sentryVitePlugin({
applicationKey,
moduleMetadata,
// Priority: top-level options > deprecated options > env vars
// eslint-disable-next-line typescript/no-deprecated
org: org ?? uploadOptions.org ?? env.SENTRY_ORG,
Expand All @@ -121,12 +123,8 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
metaFramework: 'astro',
},
},
...unstableMerged_sentryVitePluginOptions,
debug: debug ?? false,
release: {
...unstableMerged_sentryVitePluginOptions?.release,
...release,
},
release,
sourcemaps: {
...sourcemaps,
// eslint-disable-next-line typescript/no-deprecated
Expand All @@ -136,11 +134,9 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => {
// eslint-disable-next-line typescript/no-deprecated
uploadOptions?.filesToDeleteAfterUpload ??
updatedFilesToDeleteAfterUpload,
...unstableMerged_sentryVitePluginOptions?.sourcemaps,
Comment thread
chargome marked this conversation as resolved.
},
bundleSizeOptimizations: {
...bundleSizeOptimizations,
...unstableMerged_sentryVitePluginOptions?.bundleSizeOptimizations,
},
}),
],
Expand Down
20 changes: 1 addition & 19 deletions packages/astro/src/integration/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { BuildTimeOptionsBase, UnstableVitePluginOptions } from '@sentry/core';
import type { SentryVitePluginOptions } from '@sentry/bundler-plugins/vite';
import type { BuildTimeOptionsBase } from '@sentry/core';
import type { RouteData } from 'astro';

type SdkInitPaths = {
Expand Down Expand Up @@ -97,22 +96,6 @@ type SourceMapsOptions = {
* @deprecated Use `sourcemaps.filesToDeleteAfterUpload` instead
*/
filesToDeleteAfterUpload?: string | Array<string>;

/**
* Options to further customize the Sentry Vite Plugin (@sentry/bundler-plugins/vite) behavior directly.
* Options specified in this object take precedence over all other options.
*
* @see https://www.npmjs.com/package/@sentry/vite-plugin/v/2.14.2#options which lists all available options.
*
* Warning: Options within this object are subject to change at any time.
* We DO NOT guarantee semantic versioning for these options, meaning breaking
* changes can occur at any time within a major SDK version.
*
* Furthermore, some options are untested with Astro specifically. Use with caution.
*
* @deprecated Use top-level `unstable_sentryVitePluginOptions` instead
*/
unstable_sentryVitePluginOptions?: Partial<SentryVitePluginOptions>;
};

type InstrumentationOptions = {
Expand Down Expand Up @@ -164,7 +147,6 @@ type SdkEnabledOptions = {
* If you specify a dedicated init file, the SDK options passed to `sentryAstro` will be ignored for init.
*/
export type SentryOptions = BuildTimeOptionsBase &
UnstableVitePluginOptions<SentryVitePluginOptions> &
SdkInitPaths &
InstrumentationOptions &
SdkEnabledOptions & {
Expand Down
44 changes: 5 additions & 39 deletions packages/astro/test/buildOptions.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,6 @@ describe('Sentry Astro build-time options type', () => {
excludeReplayWorker: true,
},

// --- UnstableVitePluginOptions ---
unstable_sentryVitePluginOptions: {
sourcemaps: {
assets: './dist/**/*',
},
bundleSizeOptimizations: {
excludeDebugStatements: true,
},
},

// --- SentryOptions specific options ---
enabled: true,
clientInitPath: './src/sentry.client.config.ts',
Expand All @@ -84,15 +74,6 @@ describe('Sentry Astro build-time options type', () => {
autoInstrumentation: {
requestHandler: true,
},
unstable_sentryVitePluginOptions: {
sourcemaps: {
assets: './dist/**/*',
},
bundleSizeOptimizations: {
excludeDebugStatements: true,
},
},

// Deprecated sourceMapsUploadOptions
sourceMapsUploadOptions: {
enabled: true,
Expand All @@ -102,11 +83,6 @@ describe('Sentry Astro build-time options type', () => {
telemetry: false,
assets: './build/**/*',
filesToDeleteAfterUpload: ['./build/*.map'],
unstable_sentryVitePluginOptions: {
sourcemaps: {
ignore: ['./build/*.spec.js'],
},
},
},
};

Expand Down Expand Up @@ -159,24 +135,14 @@ describe('Sentry Astro build-time options type', () => {
expectTypeOf(baseOptions).toEqualTypeOf<SentryOptions>();
});

it('supports UnstableVitePluginOptions at top level', () => {
const viteOptions: SentryOptions = {
it('rejects the removed `unstable_sentryVitePluginOptions`', () => {
const options: SentryOptions = {
// @ts-expect-error - removed in v11, use the top-level build options instead
unstable_sentryVitePluginOptions: {
org: 'override-org',
project: 'override-project',
sourcemaps: {
assets: './custom-dist/**/*',
ignore: ['./custom-dist/ignore/**/*'],
},
bundleSizeOptimizations: {
excludeDebugStatements: true,
excludeTracing: false,
},
debug: true,
silent: false,
sourcemaps: { assets: './dist/**/*' },
},
};

expectTypeOf(viteOptions).toEqualTypeOf<SentryOptions>();
expectTypeOf(options).toEqualTypeOf<SentryOptions>();
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The type test for unstable_sentryVitePluginOptions is misleading. expectTypeOf(options).toEqualTypeOf<SentryOptions>() passes trivially because options is explicitly annotated, not because the extra property is correctly handled.
Severity: LOW

Suggested Fix

Remove the expectTypeOf(options).toEqualTypeOf<SentryOptions>() assertion. The @ts-expect-error comment is sufficient on its own to test and document that TypeScript correctly flags the deprecated property as an error.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/astro/test/buildOptions.test-d.ts#L147

Potential issue: The type test at `buildOptions.test-d.ts:147` is intended to verify
that the deprecated `unstable_sentryVitePluginOptions` property is rejected by the
`SentryOptions` type. However, the test is flawed. It uses `@ts-expect-error` to
suppress the type error on assignment, but then asserts
`expectTypeOf(options).toEqualTypeOf<SentryOptions>()`. Because the `options` variable
is explicitly annotated with the `SentryOptions` type, this assertion simply compares
the type to itself and always passes. It does not actually verify that an object with
the extra property is considered a different type, making the test ineffective and
providing a false sense of security.

Did we get this right? 👍 / 👎 to inform future reviews.

});
76 changes: 30 additions & 46 deletions packages/astro/test/integration/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,57 +259,41 @@ describe('sentryAstro integration', () => {
);
});

it('prefers user-specified unstable vite plugin options and merges them with default values', async () => {
// No `@ts-expect-error` here on purpose: `SentryOptions` intersects `Record<string, unknown>`, so
// TypeScript accepts any key and this runtime warning is the only signal an Astro user ever gets.
it('warns for the removed option nested inside `sourceMapsUploadOptions`', async () => {
const integration = sentryAstro({
bundleSizeOptimizations: {
excludeReplayShadowDom: true,
},
sourceMapsUploadOptions: {
enabled: true,
org: 'my-org',
project: 'my-project',
assets: ['dist/server/**/*, dist/client/**/*'],
unstable_sentryVitePluginOptions: {
org: 'my-other-org',
project: 'my-other-project',
applicationKey: 'my-application-key',
sourcemaps: {
assets: ['foo/*.js'],
ignore: ['bar/*.js'],
},
bundleSizeOptimizations: {
excludeReplayIframe: true,
},
},
},
// @ts-expect-error - removed in v11
sourceMapsUploadOptions: { unstable_sentryVitePluginOptions: { org: 'my-other-org' } },
});
// @ts-expect-error - the hook exists, and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({
...baseConfigHookObject,
updateConfig,
injectScript,
// @ts-expect-error - only passing in partial config
config: {
outDir: new URL('file://path/to/project/build'),
},
await integration.hooks['astro:config:setup']({ ...baseConfigHookObject, updateConfig, injectScript, config });

expect(baseConfigHookObject.logger.warn).toHaveBeenCalledWith(
expect.stringContaining('unstable_sentryVitePluginOptions'),
);
});

it('forwards moduleMetadata to the vite plugin', async () => {
const integration = sentryAstro({ moduleMetadata: { team: 'sdk' } });
// @ts-expect-error - the hook exists, and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({ ...baseConfigHookObject, updateConfig, injectScript, config });

expect(sentryVitePluginSpy).toHaveBeenCalledWith(expect.objectContaining({ moduleMetadata: { team: 'sdk' } }));
});

// TypeScript rejects the key (see `buildOptions.test-d.ts`); this covers JS configs, which get no
// type checking.
it('warns via the Astro logger when the removed `unstable_sentryVitePluginOptions` is still set', async () => {
const integration = sentryAstro({
// @ts-expect-error - removed in v11
unstable_sentryVitePluginOptions: { org: 'my-other-org' },
});
// @ts-expect-error - the hook exists, and we only need to pass what we actually use
await integration.hooks['astro:config:setup']({ ...baseConfigHookObject, updateConfig, injectScript, config });

expect(sentryVitePluginSpy).toHaveBeenCalledTimes(1);
expect(sentryVitePluginSpy).toHaveBeenCalledWith(
expect.objectContaining({
org: 'my-other-org',
project: 'my-other-project',
applicationKey: 'my-application-key',
sourcemaps: {
assets: ['foo/*.js'],
ignore: ['bar/*.js'],
filesToDeleteAfterUpload: ['./dist/**/client/**/*.map', './dist/**/server/**/*.map'],
},
bundleSizeOptimizations: {
excludeReplayShadowDom: true,
excludeReplayIframe: true,
},
}),
expect(baseConfigHookObject.logger.warn).toHaveBeenCalledWith(
expect.stringContaining('unstable_sentryVitePluginOptions'),
);
});

Expand Down
Loading