Skip to content

chore: Make @sentry/conventions sideEffect free during bundling - #22015

Merged
JPeer264 merged 2 commits into
developfrom
jp/external
Jul 7, 2026
Merged

chore: Make @sentry/conventions sideEffect free during bundling#22015
JPeer264 merged 2 commits into
developfrom
jp/external

Conversation

@JPeer264

@JPeer264JPeer264 commented Jul 7, 2026

Copy link
Copy Markdown
Member

@sentry/conventions are marked with sideEffects. That leads to having @sentry/conventions/attributes as side effect import in the bundled output:

Screenshot 2026-07-07 at 13 46 44

By telling rollup that @sentry/conventions is side effect free, the side effect import from the output is being removed (it also reflects the package.json#sideEffect field

Screenshot 2026-07-07 at 13 47 14

Originally I came to this fix as the Cloudflare integration tests were quite verbose in this warning:

Screenshot 2026-07-07 at 13 50 42

@JPeer264
JPeer264 requested review from a team, Lms24 and timfishJuly 7, 2026 11:49
@JPeer264JPeer264 self-assigned this Jul 7, 2026
@JPeer264
JPeer264 requested review from logaretm and mydea and removed request for a teamJuly 7, 2026 11:49
@JPeer264
JPeer264 requested a review from andreiborzaJuly 7, 2026 11:54

@Lms24Lms24 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.

thanks for fixing!

@JPeer264
JPeer264 merged commit f7c75e7 into developJul 7, 2026
297 of 299 checks passed
@JPeer264
JPeer264 deleted the jp/external branch July 7, 2026 12:27
JPeer264 added a commit that referenced this pull request Sep 2, 2026
…false` externals (#23674)
For CF integration tests there were a lot of warnings printed: ```
▲ [WARNING] Ignoring this import because ".../packages/core/build/esm/index.js"
was marked as having no side effects [ignored-bare-import]
packages/cloudflare/build/esm/prod/sdk.js:3:7:
3 │ import '@sentry/core';
```
### Root cause
`treeshake.moduleSideEffects` in
`dev-packages/rollup-utils/npmHelpers.mjs` told Rollup that every
external module has side effects. When Rollup tree-shakes away all named
bindings of such an external, it keeps a bare side-effect import to
preserve those effects. That import contradicts the dependency's own
`"sideEffects": false`, which is what the consuming bundler reports.
The Cloudflare instance is reachable from a single line in
`packages/cloudflare/src/sdk.ts`:
```ts
export { _clearGlobalClientCache } from './clientCache';
```
_clearGlobalClientCache is a test-only helper that no entrypoint
reaches, so Rollup drops the re-export. clientCache.ts imports
GLOBAL_OBJ from @sentry/core, and Rollup hoists that external dependency
into sdk.js as a bare import to keep its supposed side effects alive.
#22015 already hit this and special-cased @sentry/conventions. That
patched one instance rather than the mechanism, so the next package to
trip it, @sentry/core, brought the warnings straight back.
### Solution
Now we check for the key `sideEffects` in the respesctitive
`package.json`. If there is `sideEffects` set it will take its value,
and if there is non then we assume there are side effects, just as
before. With that the warnings are gone.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto 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

@JPeer264@Lms24