diff --git a/dev-packages/e2e-tests/test-applications/react-router-8-framework/vite.cloudflare.config.ts b/dev-packages/e2e-tests/test-applications/react-router-8-framework/vite.cloudflare.config.ts index 8a905ba1e382..002bb3a77fbe 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-8-framework/vite.cloudflare.config.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-8-framework/vite.cloudflare.config.ts @@ -10,8 +10,7 @@ export default defineConfig(async config => ({ reactRouter(), sentryCloudflareVitePlugin(), ...((await sentryReactRouter( - // Both Sentry plugins inject the orchestrion snippet, and injecting it twice fails the build. - { sourcemaps: { disable: true }, buildTimeInstrumentation: false }, + { sourcemaps: { disable: true } }, config, // eslint-disable-next-line @typescript-eslint/no-explicit-any )) as any[]), diff --git a/packages/server-utils/src/orchestrion/bundler/vite.ts b/packages/server-utils/src/orchestrion/bundler/vite.ts index 0b8f66d961f4..d199f7224385 100644 --- a/packages/server-utils/src/orchestrion/bundler/vite.ts +++ b/packages/server-utils/src/orchestrion/bundler/vite.ts @@ -9,6 +9,13 @@ import { resolveOrchestrionRuntimeRequest, SNIPPET_IMPORT_SPECIFIER } from './re type TransformHandler = (this: unknown, code: string, id: string, opts?: { ssr?: boolean }) => unknown; +// Key of a property on the plugin object. Its value is an object that is unique to each `sentryOrchestrionPlugin()` +// call. An SDK that wraps the plugin with an object spread (`@sentry/remix`) copies the property, so `configResolved` +// can find the first instance in a build also through a wrapper. +const INSTANCE_KEY = '__sentryOrchestrionInstance'; + +type OrchestrionVitePlugin = Plugin & { [INSTANCE_KEY]?: object }; + // On Vite >= 6 `applyToEnvironment` (below) keeps the whole plugin out of // client environments. Vite 5 (e.g. Remix v2) ignores that hook, so without // this gate the transform would also run in the CLIENT build — where modules @@ -16,10 +23,10 @@ type TransformHandler = (this: unknown, code: string, id: string, opts?: { ssr?: // snippet's import of the subscriber factories (which import // `node:diagnostics_channel`) breaks against Vite's browser builtin shim. Gate // on the `ssr` flag, which Vite passes on both major versions. -function ssrOnlyTransform(transform: Plugin['transform']): Plugin['transform'] { +function ssrOnlyTransform(transform: Plugin['transform'], isDuplicate: () => boolean): Plugin['transform'] { const gate = (handler: TransformHandler): TransformHandler => function (code, id, opts) { - if (!opts?.ssr) { + if (!opts?.ssr || isDuplicate()) { return null; } return handler.call(this, code, id, opts); @@ -62,9 +69,14 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { '@sentry/server-utils', ]; - return { + const instance = {}; + // `true` when an earlier instance of this plugin is in the same build, which then transforms the modules alone. + let isDuplicate = false; + + const plugin: OrchestrionVitePlugin = { ...upstream, - transform: ssrOnlyTransform(upstream.transform), + [INSTANCE_KEY]: instance, + transform: ssrOnlyTransform(upstream.transform, () => isDuplicate), // The module-injected snippet imports `@sentry/server-utils` from INSIDE // transformed `node_modules` files. Under isolated installs (pnpm) that bare // specifier doesn't resolve from an instrumented package's location, so when @@ -119,8 +131,13 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { return { resolve: { noExternal: noExternalModules() } }; }, configResolved(config: ResolvedConfig): void { + // Two Sentry Vite plugins can each add this plugin to one build, for example `sentryCloudflareVitePlugin` and + // `sentryReactRouter`. A module transformed twice declares the injected snippet twice and fails the build. + const first = (config.plugins as readonly OrchestrionVitePlugin[]).find(p => p[INSTANCE_KEY]); + isDuplicate = first !== undefined && first[INSTANCE_KEY] !== instance; + // Nothing is force-bundled in `serve`, so an externalized module is expected there. - if (config.command === 'serve') { + if (isDuplicate || config.command === 'serve') { return; } @@ -140,4 +157,6 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { } }, }; + + return plugin; } diff --git a/packages/server-utils/test/orchestrion/bundler.test.ts b/packages/server-utils/test/orchestrion/bundler.test.ts index b04fa7d1f160..59c74f664ef4 100644 --- a/packages/server-utils/test/orchestrion/bundler.test.ts +++ b/packages/server-utils/test/orchestrion/bundler.test.ts @@ -227,6 +227,7 @@ describe('sentryOrchestrionPlugin (vite)', () => { const plugin = vitePlugin(); (plugin.configResolved as (config: unknown) => void)({ command, + plugins: [plugin], ssr: { external: ssrExternal }, logger: { warn }, } as unknown as ResolvedConfig); @@ -324,6 +325,59 @@ describe('sentryOrchestrionPlugin (vite)', () => { expect(transform.call({}, 'code', 'id', { ssr: true })).toBe('transformed'); }); + it('transforms and warns only in the first instance when two Sentry plugins add it to one build', () => { + const first = vitePlugin(); + const second = vitePlugin(); + const warn = vi.fn(); + const config = { + command: 'build', + plugins: [{ name: 'other' }, first, second], + ssr: { external: ['mysql'] }, + logger: { warn }, + } as unknown as ResolvedConfig; + + (first.configResolved as (config: unknown) => void)(config); + (second.configResolved as (config: unknown) => void)(config); + + const firstTransform = first.transform as ( + this: unknown, + code: string, + id: string, + opts?: { ssr?: boolean }, + ) => unknown; + const secondTransform = second.transform as ( + this: unknown, + code: string, + id: string, + opts?: { ssr?: boolean }, + ) => unknown; + expect(firstTransform.call({}, 'code', 'id', { ssr: true })).toBe('transformed'); + expect(secondTransform.call({}, 'code', 'id', { ssr: true })).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('still transforms when an SDK wraps the plugin with an object spread', () => { + // `@sentry/remix` puts `{ ...orchestrion, configResolved, transform }` into the build and calls the original hooks. + const plugin = vitePlugin(); + const wrapper = { ...plugin, configResolved: vi.fn(), transform: vi.fn() }; + const config = { + command: 'build', + plugins: [wrapper], + ssr: {}, + logger: { warn: vi.fn() }, + } as unknown as ResolvedConfig; + + (plugin.configResolved as (config: unknown) => void)(config); + + const transform = plugin.transform as ( + this: unknown, + code: string, + id: string, + opts?: { ssr?: boolean }, + ) => unknown; + expect(transform.call({}, 'code', 'id', { ssr: true })).toBe('transformed'); + }); + it('gates resolveId on the ssr flag and falls back to self-resolution', async () => { const plugin = vitePlugin(); const resolveId = plugin.resolveId as (