Skip to content

feat(node): Ensure modulesIntegration works in more environments #16566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 13, 2025

Conversation

mydea
Copy link
Member

@mydea mydea commented Jun 12, 2025

Extracted out from #16565

I noticed that our modulesIntegration is pretty limited:

  1. It does nothing on EMS
  2. It does not work on Next.js (even though that is CJS)

This PR makes this a bit more robust (not perfect):

  1. It generally now also tries to look at process.cwd() + 'package.json' and take the dependencies and devDependencies from there. this should generally work in esm apps now as well, at least at a basic level. You do not get all dependencies and versions may be ranges, but better than nothing.
  2. For Next.js, we inject a modules list based off the package.json at build time, as we do not have proper access to this at runtime.

@mydea mydea requested review from Lms24 and AbhiPrasad June 12, 2025 12:28
@mydea mydea self-assigned this Jun 12, 2025
@mydea mydea force-pushed the fn/better-modules-integration branch from b9ac82d to 7f29495 Compare June 12, 2025 12:28
Copy link
Contributor

github-actions bot commented Jun 12, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.99 kB - -
@sentry/browser - with treeshaking flags 23.76 kB - -
@sentry/browser (incl. Tracing) 38.69 kB - -
@sentry/browser (incl. Tracing, Replay) 76.78 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 69.87 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 81.54 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 93.6 kB - -
@sentry/browser (incl. Feedback) 40.73 kB - -
@sentry/browser (incl. sendFeedback) 28.7 kB - -
@sentry/browser (incl. FeedbackAsync) 33.59 kB - -
@sentry/react 25.76 kB - -
@sentry/react (incl. Tracing) 40.67 kB - -
@sentry/vue 28.36 kB - -
@sentry/vue (incl. Tracing) 40.55 kB - -
@sentry/svelte 24.01 kB - -
CDN Bundle 25.48 kB - -
CDN Bundle (incl. Tracing) 38.77 kB - -
CDN Bundle (incl. Tracing, Replay) 74.64 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 80.1 kB - -
CDN Bundle - uncompressed 74.48 kB - -
CDN Bundle (incl. Tracing) - uncompressed 114.89 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 228.86 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 241.69 kB - -
@sentry/nextjs (client) 42.33 kB - -
@sentry/sveltekit (client) 39.17 kB - -
@sentry/node 150.5 kB +0.02% +25 B 🔺
@sentry/node - without tracing 98.38 kB +0.04% +31 B 🔺
@sentry/aws-serverless 124.14 kB +0.03% +33 B 🔺

View base workflow run

Comment on lines +137 to +139
...packageJson.dependencies,
...packageJson.devDependencies,
Copy link
Member

Choose a reason for hiding this comment

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

l/maybe something to follow up on in the future: Would be cool to add some kind of flag to differ between dependencies and devDependencies. Doesn't have to happen now of course

@mydea mydea force-pushed the fn/better-modules-integration branch from 96b9078 to 988111d Compare June 12, 2025 15:05
@mydea mydea force-pushed the fn/better-modules-integration branch from 988111d to cec19a7 Compare June 12, 2025 15:17
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

There's probably a bunch of edge cases here with monorepos and mixed esm/cjs environments that we don't account for, but I think this is a good starting point.

@mydea mydea merged commit c92a9b0 into develop Jun 13, 2025
145 checks passed
@mydea mydea deleted the fn/better-modules-integration branch June 13, 2025 06:36
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.

3 participants