Skip to content

Commit 8583d50

Browse files
JPeer264claude
andcommitted
fix(server-utils): Transform modules once when two Sentry Vite plugins add orchestrion
`sentryCloudflareVitePlugin` and a framework plugin such as `sentryReactRouter` both add the orchestrion Vite plugin to one build. Both instances then transform each instrumented module, the second one declares the injected snippet again, and the build fails with a redeclaration error. Each plugin object now carries an ID property, and an instance that finds an earlier plugin with another ID in the same build leaves the transform to it. The ID survives SDKs that wrap the plugin with an object spread, such as `@sentry/remix`. `react-router-8-framework` no longer needs `buildTimeInstrumentation: false` in its Cloudflare config. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9d6d6f9 commit 8583d50

3 files changed

Lines changed: 79 additions & 7 deletions

File tree

‎dev-packages/e2e-tests/test-applications/react-router-8-framework/vite.cloudflare.config.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ export default defineConfig(async config => ({
1010
reactRouter(),
1111
sentryCloudflareVitePlugin(),
1212
...((await sentryReactRouter(
13-
// Both Sentry plugins inject the orchestrion snippet, and injecting it twice fails the build.
14-
{ sourcemaps: { disable: true }, buildTimeInstrumentation: false },
13+
{ sourcemaps: { disable: true } },
1514
config,
1615
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1716
)) as any[]),

‎packages/server-utils/src/orchestrion/bundler/vite.ts‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,24 @@ import { resolveOrchestrionRuntimeRequest, SNIPPET_IMPORT_SPECIFIER } from './re
99

1010
type TransformHandler = (this: unknown, code: string, id: string, opts?: { ssr?: boolean }) => unknown;
1111

12+
// Key of a property on the plugin object. Its value is an object that is unique to each `sentryOrchestrionPlugin()`
13+
// call. An SDK that wraps the plugin with an object spread (`@sentry/remix`) copies the property, so `configResolved`
14+
// can find the first instance in a build also through a wrapper.
15+
const INSTANCE_KEY = '__sentryOrchestrionInstance';
16+
17+
type OrchestrionVitePlugin = Plugin & { [INSTANCE_KEY]?: object };
18+
1219
// On Vite >= 6 `applyToEnvironment` (below) keeps the whole plugin out of
1320
// client environments. Vite 5 (e.g. Remix v2) ignores that hook, so without
1421
// this gate the transform would also run in the CLIENT build — where modules
1522
// like `@remix-run/server-runtime` sit in the client graph, and the injected
1623
// snippet's import of the subscriber factories (which import
1724
// `node:diagnostics_channel`) breaks against Vite's browser builtin shim. Gate
1825
// on the `ssr` flag, which Vite passes on both major versions.
19-
function ssrOnlyTransform(transform: Plugin['transform']): Plugin['transform'] {
26+
function ssrOnlyTransform(transform: Plugin['transform'], isDuplicate: () => boolean): Plugin['transform'] {
2027
const gate = (handler: TransformHandler): TransformHandler =>
2128
function (code, id, opts) {
22-
if (!opts?.ssr) {
29+
if (!opts?.ssr || isDuplicate()) {
2330
return null;
2431
}
2532
return handler.call(this, code, id, opts);
@@ -62,9 +69,14 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin {
6269
'@sentry/server-utils',
6370
];
6471

65-
return {
72+
const instance = {};
73+
// `true` when an earlier instance of this plugin is in the same build, which then transforms the modules alone.
74+
let isDuplicate = false;
75+
76+
const plugin: OrchestrionVitePlugin = {
6677
...upstream,
67-
transform: ssrOnlyTransform(upstream.transform),
78+
[INSTANCE_KEY]: instance,
79+
transform: ssrOnlyTransform(upstream.transform, () => isDuplicate),
6880
// The module-injected snippet imports `@sentry/server-utils` from INSIDE
6981
// transformed `node_modules` files. Under isolated installs (pnpm) that bare
7082
// specifier doesn't resolve from an instrumented package's location, so when
@@ -119,8 +131,13 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin {
119131
return { resolve: { noExternal: noExternalModules() } };
120132
},
121133
configResolved(config: ResolvedConfig): void {
134+
// Two Sentry Vite plugins can each add this plugin to one build, for example `sentryCloudflareVitePlugin` and
135+
// `sentryReactRouter`. A module transformed twice declares the injected snippet twice and fails the build.
136+
const first = (config.plugins as readonly OrchestrionVitePlugin[]).find(p => p[INSTANCE_KEY]);
137+
isDuplicate = first !== undefined && first[INSTANCE_KEY] !== instance;
138+
122139
// Nothing is force-bundled in `serve`, so an externalized module is expected there.
123-
if (config.command === 'serve') {
140+
if (isDuplicate || config.command === 'serve') {
124141
return;
125142
}
126143

@@ -140,4 +157,6 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin {
140157
}
141158
},
142159
};
160+
161+
return plugin;
143162
}

‎packages/server-utils/test/orchestrion/bundler.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ describe('sentryOrchestrionPlugin (vite)', () => {
227227
const plugin = vitePlugin();
228228
(plugin.configResolved as (config: unknown) => void)({
229229
command,
230+
plugins: [plugin],
230231
ssr: { external: ssrExternal },
231232
logger: { warn },
232233
} as unknown as ResolvedConfig);
@@ -324,6 +325,59 @@ describe('sentryOrchestrionPlugin (vite)', () => {
324325
expect(transform.call({}, 'code', 'id', { ssr: true })).toBe('transformed');
325326
});
326327

328+
it('transforms and warns only in the first instance when two Sentry plugins add it to one build', () => {
329+
const first = vitePlugin();
330+
const second = vitePlugin();
331+
const warn = vi.fn();
332+
const config = {
333+
command: 'build',
334+
plugins: [{ name: 'other' }, first, second],
335+
ssr: { external: ['mysql'] },
336+
logger: { warn },
337+
} as unknown as ResolvedConfig;
338+
339+
(first.configResolved as (config: unknown) => void)(config);
340+
(second.configResolved as (config: unknown) => void)(config);
341+
342+
const firstTransform = first.transform as (
343+
this: unknown,
344+
code: string,
345+
id: string,
346+
opts?: { ssr?: boolean },
347+
) => unknown;
348+
const secondTransform = second.transform as (
349+
this: unknown,
350+
code: string,
351+
id: string,
352+
opts?: { ssr?: boolean },
353+
) => unknown;
354+
expect(firstTransform.call({}, 'code', 'id', { ssr: true })).toBe('transformed');
355+
expect(secondTransform.call({}, 'code', 'id', { ssr: true })).toBeNull();
356+
expect(warn).toHaveBeenCalledTimes(1);
357+
});
358+
359+
it('still transforms when an SDK wraps the plugin with an object spread', () => {
360+
// `@sentry/remix` puts `{ ...orchestrion, configResolved, transform }` into the build and calls the original hooks.
361+
const plugin = vitePlugin();
362+
const wrapper = { ...plugin, configResolved: vi.fn(), transform: vi.fn() };
363+
const config = {
364+
command: 'build',
365+
plugins: [wrapper],
366+
ssr: {},
367+
logger: { warn: vi.fn() },
368+
} as unknown as ResolvedConfig;
369+
370+
(plugin.configResolved as (config: unknown) => void)(config);
371+
372+
const transform = plugin.transform as (
373+
this: unknown,
374+
code: string,
375+
id: string,
376+
opts?: { ssr?: boolean },
377+
) => unknown;
378+
expect(transform.call({}, 'code', 'id', { ssr: true })).toBe('transformed');
379+
});
380+
327381
it('gates resolveId on the ssr flag and falls back to self-resolution', async () => {
328382
const plugin = vitePlugin();
329383
const resolveId = plugin.resolveId as (

0 commit comments

Comments
 (0)