Repository navigation
Conversation
96b134e to
9d7eca2
Compare
…s 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>
9d7eca2 to
8583d50
Compare
andreiborza
left a comment
There was a problem hiding this comment.
I think this approach is prone to headaches. Plugins winning over others and completely disabling them ends up with issues at runtime. For example what happens when deploying on cloudflare but the sentryCloudflareVitePlugin does not win out over the other plugins?
Won't we have issues with dc related stuff missing in that case?
I'm not sure what a good approach here would be, maybe we could assign higher priority to the cloudflare plugin because it does more? That opens up a can of maintenance worms too I imagine.
Another idea would be to do checks at transformation time and see if modules were already wrapped and bail out instead?
In this case we don't really disable one plugin over another, as React Router and Cloudflare, both, use the same orchestrion Vite plugin. Here we only make sure we don't register them twice, directly in the orchestrion layer. So even if However, it would be intense and a maintenance hell if we have a mix of esbuild + vite (which hopefully would never be the case) - now we just make sure we don't allow two registrations right after each other, which is only true for the orchestrion layer and not the auto instrumentation of Cloudflare. That means we don't disable the
That would be the same approach this PR is doing, just one level deeper (which I think is more code, as it has to live on each integration) |
closes #24836
closes JS-3837
Description
This fixes the Vite build when
sentryCloudflareVitePluginand a framework's Sentry plugin, for examplesentryReactRouter, are in the same config. Only the first orchestrion plugin in the build now transforms modules, so apps no longer needbuildTimeInstrumentation: false.Why
Both plugins add the orchestrion Vite plugin. Each instrumented module was transformed twice, and the build failed because the injected
orchestrionModuleInjectedsnippet was declared twice.Dedupe decisions
Each plugin object carries an ID property, and an instance skips its work when an earlier plugin in the build has a different ID. The ID also survives SDKs that wrap the plugin with an object spread, like
@sentry/remix. The alternative is that each framework plugin detects the Cloudflare plugin and turns its own orchestrion off.Known issues
instrumentations, only the options of the first one apply.