Skip to content

ci(core): Guard the "sideEffects": false contract with a lint check - #6836

Open
antonis wants to merge 2 commits into
mainfrom
antonis/ci-guard-side-effects
Open

antonis wants to merge 2 commits into
mainfrom
antonis/ci-guard-side-effects

Conversation

@antonis

@antonis antonis commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Adds a lint check that fails if any module in packages/core/src/js (excluding the Node-only tools/) runs code at import time.

💡 Motivation and Context

Follow-up to #6829 / getsentry/sentry#126410.

💚 How did you test it?

  • CI

📝 Checklist

  • I added tests to verify changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.
  • No breaking changes.

🔮 Next steps

PR #6829 marked @sentry/react-native as side-effect free so bundlers can
tree-shake unused exports. That flag is an unguarded, package-wide promise:
if any shipped module ever runs code at import time, a bundler may drop it
when its exports are unused, silently breaking the SDK in consumer bundles.

Add scripts/check-side-effects.js, wired into `yarn lint`, which scans
src/js (excluding Node-only tools/) and fails if a module has an import-time
side effect: a bare side-effect import, or a top-level executed statement
(call, assignment, IIFE, global write). Declarations are allowed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Semver Impact of This PR

⚪ None (no version bump detected)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


  • ci(core): Guard the "sideEffects": false contract with a lint check by antonis in #6836
  • fix(core): Mark package as side-effect free by devclaimjuimperai in #6829
  • chore(deps): bump getsentry/craft/.github/workflows/changelog-preview.yml from 2.31.0 to 2.33.1 by dependabot in #6833
  • chore(deps): bump gradle/actions/setup-gradle from 6.3.0 to 6.4.0 by dependabot in #6834
  • chore(deps): bump getsentry/craft from 2.31.2 to 2.33.1 by dependabot in #6835
  • chore(deps): bump getsentry/github-workflows/validate-pr from 4013fc6e1aeb1be1f9d3b4d232624f0ec1afa613 to 36c729264d2edc29ebae61950c50e1e9f043ad7e by dependabot in #6832
  • fix(android): Settle initNativeReactNavigationNewFrameTracking promise by antonis in #6823
  • fix(spotlight): Forward image attachments to Spotlight by antonis in #6818
  • fix(ios): Prevent crash when initialized with an invalid DSN by antonis in #6825
  • chore(core): Resolve non-actionable TODOs by antonis in #6826
  • chore(core): resolve stale TODO comments by antonis in #6819
  • fix(profiling): Populate Hermes runtime version on JS profiles by antonis in #6817

🤖 This preview updates automatically when you update the PR.

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor
Fails
🚫 Pull request is not ready for merge, please add the "ready-to-merge" label to the pull request

Generated by 🚫 dangerJS against aabb031

Comment thread packages/core/scripts/check-side-effects.js

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread packages/core/scripts/check-side-effects.js
Review feedback (Seer + Cursor Bugbot): the scan only flagged top-level
ExpressionStatements and clause-less imports, so a side effect wrapped in an
`if`/`for`/`while`/`try`/block — or an empty-binding `import {} from 'x'` —
slipped through while the lint stayed green.

Invert to an allowlist: permit only pure declarations at module top level
(function/class/interface/type/enum/namespace/variable/export/import-equals),
flag every other statement, and treat an import that binds nothing (incl.
`import {} from 'x'`) as a side-effect import. Type-only imports are erased,
so they're allowed. Exclude `vendor/` (audited third-party code) alongside
`tools/`, so the guard targets first-party SDK code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit aabb031. Configure here.

@antonis
antonis marked this pull request as ready for review October 5, 2026 08:28
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.

1 participant