Repository navigation
Conversation
Contributor
size-limit report 📦Compared: 6ba76e3 (PR test merge using develop at da369a5) Against: 9c93bbd (develop) · base run #64212
|
Contributor
Optional E2E testsSome optional checks were not successful. They do not block the merge. 1 not successful, 56 successful. See the run.
Successful checks
|
isaacs
reviewed
Oct 27, 2026
isaacs
left a comment
isaacs
left a comment
Member
There was a problem hiding this comment.
The design is sound for its target. The main problem is scope. It applies to every server environment of every SDK that uses the shared Vite plugin, not only cloudflare.
Comment on lines
+92
to
+94
| if (isDuplicate()) { | ||
| return; | ||
| } |
Member
There was a problem hiding this comment.
This is function is gonna cause a crash if it's used in a node bundle with vite.
On Vite 6 and 7, the dependency optimizer uses platform: 'node' for any server environment that is not webworker (the default ssr environment of every Node SSR framework). For that platform, Vite adds this banner to every optimized file:
import { createRequire } from 'module';const require = createRequire(import.meta.url);The new esbuild plugin makes the optimizer bundle @sentry/server-utils (and @sentry/core) into the pre-bundled dependency.
Two lines in depOptimizerEsbuildPlugin cause this. Line 112 (upstream.setup(build)) adds the transform, which puts an import of @sentry/server-utils into each instrumented module. Line 107 then resolves that import to a file path (toEsmEntry(resolved.path)), so esbuild bundles the file instead of leaving the import external. The @sentry/server-utils ESM build has its own top-level import { createRequire } from "module", and esbuild cannot rename around a text banner. The result is a file that doesn't parse:
vite 6.4.3 esm load error: Identifier 'createRequire' has already been declared
vite 6.4.3 cjs load error: Identifier 'createRequire' has already been declared
vite 7.3.6 esm load error: Identifier 'createRequire' has already been declared
vite 7.3.6 cjs load error: Identifier 'createRequire' has already been declared
vite 8.3.1 esm runs: x channel fired: true
vite 8.3.1 cjs runs: x channel fired: true
Cloudflare is not affected, because @cloudflare/vite-plugin sets platform: 'neutral', which has no banner. With platform: 'neutral' the Vite 6/7 ESM case loads and fires the channel.
Suggestion: scope the dev optimizer plugins to the case this PR targets. Two options:
- Minimal: don't register the resolver or the transform when the optimizer bundles for Node. Vite passes the platform it picked to esbuild as
build.initialOptions.platform, sosetup(line 91) can return before it reaches lines 96-112. The existing duplicate-instance check here already returns early in the same place, so the new check can go next to it:
Suggested change
| if (isDuplicate()) { | |
| return; | |
| } | |
| // Vite 6/7 add a `createRequire` banner for `platform: 'node'`, which | |
| // collides with the `createRequire` import in the bundled SDK. | |
| if (build.initialOptions.platform === 'node') { | |
| return; | |
| } | |
| if (isDuplicate()) { | |
| return; | |
| } |
- Cleaner: add an opt-in option to
PluginOptionsthatsentryCloudflareVitePluginsets, so the other SDKs keep today'sservebehavior. The PR title saysfix(cloudflare), and the comment at lines 205-207 names@cloudflare/vite-pluginas the reason. An opt-in would make the change match that scope. It would also avoid the side effects on Node SSR envs. This option touches three places: the new field inPluginOptions(packages/server-utils/src/orchestrion/bundler/options.ts), theservebranch at line 208 ofvite.ts(returnnullwhen the option is off), and thesentryOrchestrionPlugin({ ... })call atpackages/cloudflare/src/vite/index.tsline 88.
Member
Author
There was a problem hiding this comment.
I'm gonna check. I like the opt-in method.
| const plugin = vitePlugin(); | ||
| // Vite 6 passes no context, Vite 8 passes `rolldownVersion`. | ||
| context?: { meta: { rolldownVersion?: string } }, | ||
| plugin = vitePlugin(), |
Member
There was a problem hiding this comment.
nit: this now takes 5 positional parameters. Might be time for an options object?
Member
Author
There was a problem hiding this comment.
Yeah that's actually better! I'll change it
JPeer264
marked this pull request as ready for review
October 27, 2026 22:10
JPeer264
requested review from
andreiborza,
isaacs and
mydea
and removed request for
a team and
mydea
October 27, 2026 22:11
Member
Author
Good that you brought that up. I actually assumed there might be cases where it is applicable in Node as well. Since on plain Vite+Node it's already working we can do the opt-in as you suggested. |
Lms24
approved these changes
Oct 29, 2026
Lms24
requested review from
chargome,
logaretm,
msonnb and
nicohrubec
and removed request for
a team
October 29, 2026 16:06
Lms24
force-pushed
the
jp/cloudflare-vite-dev-instrumentation
branch
from
October 29, 2026 16:06
76f66b4 to
02226bf
Compare
`@cloudflare/vite-plugin` pre-bundles the worker dependencies in `vite dev`, and the Vite dependency optimizer runs no Vite plugins. So the orchestrion transform never ran on instrumented packages, and their spans were missing. The orchestrion Vite plugin now adds the transform to `optimizeDeps` of server environments in `serve`: a Rolldown plugin on Vite 8 and an esbuild plugin on Vite 6 and 7. Both resolve the snippet import to the ESM entry of `@sentry/server-utils`, like the build does. The cloudflare variant of the `hono-4` e2e app now also runs its tests against `vite dev`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lms24
force-pushed
the
jp/cloudflare-vite-dev-instrumentation
branch
from
October 29, 2026 17:26
02226bf to
5a5f947
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
part of #25765
part of JS-4010
Description
This runs the orchestrion transform in the Vite dependency optimizer, so integrations that are added at build time (for example
honoandgraphql) also send spans invite devon Cloudflare.@cloudflare/vite-pluginpre-bundles the Worker dependencies intonode_modules/.vite/deps_*, and before, the transform did not run on these files. The plugin adds a Rolldown plugin for Vite 8 and an esbuild plugin for Vite 6 and 7.The new
transformOptimizedDepsplugin option turns this on, andsentryCloudflareVitePluginsets it. Node servers do not need it, because the runtime hook instruments the dependencies that Node loads.The Cloudflare variant of the
hono-4e2e app now also runs its tests invite dev.Known issues
optimizeDepsis still not instrumented invite dev: its module id ends with?v=<hash>, and the file matcher of@apm-js-collab/code-transformer-bundler-pluginsdoes not remove the query (Vite plugin does not transform modules whose id has a query (?v=<hash> in vite dev) apm-js-collab/code-transformer-bundler-plugins#52).Sentry traces
Before (11.9.0),
/graphqlinvite dev, nographqlspans: https://sentry-sdks.sentry.io/explore/traces/trace/9221590d886f4648b401eded8a996046/After,
/graphqlinvite dev, withgraphqlspans: https://sentry-sdks.sentry.io/explore/traces/trace/2aea20834d3d4fa38e39d37166273e14/Before (11.9.0),
/hono/users/1invite dev, span nameGET /hono/users/1: https://sentry-sdks.sentry.io/explore/traces/trace/b8e5b8dba83b4b5aba45bdfdf40b307c/After,
/hono/users/1invite dev, span nameGET /hono/users/:idfrom the Hono route: https://sentry-sdks.sentry.io/explore/traces/trace/20b2a5dcfe6a4dbfb7e68eb3037bf3b2/