Skip to content

fix(node): Read module versions from the isolated layouts of pnpm and Bun - #25526

Open
JPeer264 wants to merge 2 commits into
developfrom
jp/node-modules-pnpm-isolated
Open

JPeer264 wants to merge 2 commits into
developfrom
jp/node-modules-pnpm-isolated

Conversation

@JPeer264

@JPeer264 JPeer264 commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

This fixes modulesIntegration for CJS apps that pnpm or Bun install with the isolated layout. The integration reads the version of each module in require.cache from its package.json, but only for packages directly in a node_modules folder of the app. pnpm and Bun keep them in node_modules/<.pnpm or .bun>/<package id>/node_modules/<name>, so event.modules had only the version ranges from the app's package.json. This also fixes scoped packages (@scope/name), which the integration did not read in any layout.

The store keeps every version of a package. When more than one version is loaded, event.modules has the one that require('<name>') in the main module loads.

Known issues

  • When only dependencies of the app load a package, and they load two versions of it, the version that was loaded last wins.

The new suites/modules test writes packages into these layouts, because the monorepo installs with the hoisted layout.

@JPeer264
JPeer264 added this pull request to stack #25528 October 19, 2026 13:44
@JPeer264
JPeer264 force-pushed the jp/node-modules-pnpm-isolated branch from aab2593 to 220ad81 Compare October 19, 2026 14:33
@JPeer264
JPeer264 marked this pull request as ready for review October 19, 2026 14:57
@JPeer264
JPeer264 requested a review from a team as a code owner October 19, 2026 14:57
@JPeer264
JPeer264 requested review from isaacs and mydea and removed request for a team October 19, 2026 14:57
@JPeer264 JPeer264 self-assigned this Oct 19, 2026
@JPeer264
JPeer264 requested review from andreiborza and s1gr1d and removed request for mydea October 19, 2026 14:57

@isaacs isaacs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's a surprising load-order issue, and we should perhaps add support for @scope/pkg while we're changing it.

Comment thread packages/node/src/integrations/modules.ts
Comment thread packages/node/src/integrations/modules.ts
@JPeer264
JPeer264 force-pushed the jp/node-modules-pnpm-isolated branch from 220ad81 to db2c9e6 Compare October 21, 2026 17:10
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

size-limit report 📦

Compared: 484e384 (PR test merge using develop at ef5596b)

Against: ef5596b (develop) · base run #63094

✅ Baseline matches: this diff contains only this PR's changes.

Path Size % Change Change
@sentry/browser 29.91 kB - -
@sentry/browser - with treeshaking flags 28.05 kB - -
@sentry/browser - with treeshaking flags tracing without tracing 27.95 kB - -
@sentry/browser (incl. Tracing) 52.1 kB - -
@sentry/browser (incl. Tracing + Span Streaming) 52.1 kB - -
@sentry/browser (incl. Tracing, Profiling) 55.05 kB - -
@sentry/browser (incl. Tracing, Replay) 91.85 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 80.72 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 96.58 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 109.52 kB - -
@sentry/browser (incl. Feedback) 47.42 kB - -
@sentry/browser (incl. sendFeedback) 34.97 kB - -
@sentry/browser (incl. FeedbackAsync) 40.07 kB - -
@sentry/browser (incl. Metrics) 30.93 kB - -
@sentry/browser (incl. Logs) 31.21 kB - -
@sentry/browser (incl. Metrics & Logs) 31.84 kB - -
@sentry/react 31.74 kB - -
@sentry/react (incl. Tracing) 54.42 kB - -
@sentry/vue 38.07 kB - -
@sentry/vue (incl. Tracing) 55.03 kB - -
@sentry/svelte 29.93 kB - -
@sentry/remix (Remix 3 client bundle) 57.11 kB - -
CDN Bundle 31.67 kB - -
CDN Bundle (incl. Tracing) 52.61 kB - -
CDN Bundle (incl. Logs, Metrics) 33.87 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) 54.55 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) 74.79 kB - -
CDN Bundle (incl. Tracing, Replay) 90.28 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 92.21 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 96.42 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 98.37 kB - -
CDN Bundle - uncompressed 93.29 kB - -
CDN Bundle (incl. Tracing) - uncompressed 156.14 kB - -
CDN Bundle (incl. Logs, Metrics) - uncompressed 99.84 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 162.11 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 229.88 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 276.31 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 282.26 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 290.01 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 295.96 kB - -
@sentry/nextjs (client) 57.31 kB - -
@sentry/sveltekit (client) 52.48 kB - -
@sentry/core/server 41.85 kB - -
@sentry/core/browser 13.81 kB - -
@sentry/node 152.96 kB +0.01% +1 B 🔺
@sentry/node/import (ESM hook with diagnostics-channel injection) 81.58 kB - -
@sentry/node - without tracing 94.57 kB +0.01% +1 B 🔺
@sentry/node - without channel injection 133.6 kB - -
@sentry/aws-serverless 102.74 kB +0.01% +1 B 🔺
@sentry/cloudflare (withSentry) - minified 191.69 kB - -
@sentry/cloudflare (withSentry) 476.34 kB - -
@sentry/nextjs/cloudflare (withSentry) - minified 212.65 kB - -

View base workflow run

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Optional E2E tests

Some optional checks were not successful. They do not block the merge. 1 failing, 49 successful. See the run.

Check Status
❌ CI: Optional E2E Tests / E2E node-anthropic-send-to-sentry (latest) Test (optional) Failing after 13s Details
Successful checks
Check Status
✅ CI: Optional E2E Tests / E2E nextjs-16 (vinext) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nextjs-15 (latest) Test (optional) Successful in 2m Details
✅ CI: Optional E2E Tests / E2E nextjs-16 (cloudflare, latest) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E astro-7-static Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nextjs-16-standalone (canary, turbopack) Test (optional) Successful in 25s Details
✅ CI: Optional E2E Tests / E2E nextjs-16 (canary, webpack) Test (optional) Successful in 4m Details
✅ CI: Optional E2E Tests / E2E nextjs-14 (canary) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nextjs-15 (turbo) Test (optional) Successful in 2m Details
✅ CI: Optional E2E Tests / E2E nextjs-16 (canary, turbopack) Test (optional) Successful in 3m Details
✅ CI: Optional E2E Tests / E2E astro-7 Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nuxt-4 (canary) Test (optional) Successful in 2m Details
✅ CI: Optional E2E Tests / E2E nextjs-orpc Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E gen-ai-libraries (cloudflare) Test (optional) Successful in 54s Details
✅ CI: Optional E2E Tests / E2E node-flue Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nextjs-14 (latest) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E node-fastify-6 Test (optional) Successful in 21s Details
✅ CI: Optional E2E Tests / E2E debug-id-sourcemaps Test (optional) Successful in 42s Details
✅ CI: Optional E2E Tests / E2E node-anthropic-send-to-sentry Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E node-openai-send-to-sentry (latest) Test (optional) Successful in 2m Details
✅ CI: Optional E2E Tests / E2E node-eve (latest) Test (optional) Successful in 51s Details
✅ CI: Optional E2E Tests / E2E node-eve (orchestrion) Test (optional) Successful in 42s Details
✅ CI: Optional E2E Tests / E2E node-openai-send-to-sentry Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nuxt-5-sourcemaps Test (optional) Successful in 23s Details
✅ CI: Optional E2E Tests / E2E node-mastra (latest) Test (optional) Successful in 48s Details
✅ CI: Optional E2E Tests / E2E gen-ai-libraries Test (optional) Successful in 43s Details
✅ CI: Optional E2E Tests / E2E node-pi-durable (latest) Test (optional) Successful in 25s Details
✅ CI: Optional E2E Tests / E2E nextjs-16 (deno) Test (optional) Successful in 2m Details
✅ CI: Optional E2E Tests / E2E supabase-nextjs Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E node-express-send-to-sentry-static Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nextjs-16-tunnel (cloudflare) Test (optional) Successful in 51s Details
✅ CI: Optional E2E Tests / E2E nuxt-5-sourcemaps (client source maps kept) Test (optional) Successful in 29s Details
✅ CI: Optional E2E Tests / E2E node-openai-decisions-send-to-sentry (latest) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E solidstart-2 Test (optional) Successful in 36s Details
✅ CI: Optional E2E Tests / E2E node-fastify-6 (tracer provider) Test (optional) Successful in 22s Details
✅ CI: Optional E2E Tests / E2E node-openai-decisions-send-to-sentry Test (optional) Successful in 59s Details
✅ CI: Optional E2E Tests / E2E tanstackstart-react-cloudflare Test (optional) Successful in 22s Details
✅ CI: Optional E2E Tests / E2E react-router-7-framework-instrumentation Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nuxt-4-static (canary) Test (optional) Successful in 2m Details
✅ CI: Optional E2E Tests / E2E nextjs-16 (vinext, bun) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E node-express-mcp-v2 Test (optional) Successful in 22s Details
✅ CI: Optional E2E Tests / E2E node-express-send-to-sentry Test (optional) Successful in 52s Details
✅ CI: Optional E2E Tests / E2E node-mastra Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nuxt-5-sourcemaps (static / nuxt generate) Test (optional) Successful in 25s Details
✅ CI: Optional E2E Tests / E2E nuxt-3 (canary) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E node-flue (latest) Test (optional) Successful in 1m Details
✅ CI: Optional E2E Tests / E2E nuxt-5 Test (optional) Successful in 2m Details
✅ CI: Optional E2E Tests / E2E node-express-mcp-v1 Test (optional) Successful in 17s Details
✅ CI: Optional E2E Tests / E2E node-eve Test (optional) Successful in 56s Details
✅ CI: Optional E2E Tests / E2E node-pi-durable Test (optional) Successful in 24s Details

@JPeer264 JPeer264 changed the title fix(node): Read module versions from the pnpm isolated layout fix(node): Read module versions from the isolated layouts of pnpm and Bun Oct 21, 2026
@JPeer264
JPeer264 requested a review from isaacs October 21, 2026 20:32
@isaacs
isaacs force-pushed the jp/node-modules-pnpm-isolated branch from db2c9e6 to 7515a54 Compare October 21, 2026 21:01
@JPeer264
JPeer264 force-pushed the jp/node-modules-pnpm-isolated branch 2 times, most recently from c1d41c2 to 085db24 Compare October 22, 2026 09:03
JPeer264 and others added 2 commits October 22, 2026 11:04
`modulesIntegration` reads the version of each CJS module in
`require.cache` from its package.json in a node_modules folder of the
main module. The isolated pnpm layout keeps the package files in
`node_modules/.pnpm/<package id>/node_modules/<name>`, so the
integration found no version there. Events got only the version ranges
from the app's package.json, and no packages that the app does not
declare.

Read these folders like the node_modules folders of the main module.
The new test writes a package into this layout, because the monorepo
installs its packages with the hoisted layout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@JPeer264
JPeer264 force-pushed the jp/node-modules-pnpm-isolated branch from 085db24 to 7a758c5 Compare October 22, 2026 09:11

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants