From 72e56f8f6aef46b1f1659501da8ad31ceecf9c5a Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 5 Oct 2026 10:09:02 +0200 Subject: [PATCH 1/2] ci(core): Guard the "sideEffects": false contract with a lint check 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 --- packages/core/package.json | 3 +- packages/core/scripts/check-side-effects.js | 59 +++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 packages/core/scripts/check-side-effects.js diff --git a/packages/core/package.json b/packages/core/package.json index c441082bd4..8182660c7f 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -41,8 +41,9 @@ "fix": "npx run-s fix:oxlint fix:fmt", "fix:oxlint": "OXLINT_TSGOLINT_DANGEROUSLY_SUPPRESS_PROGRAM_DIAGNOSTICS=true oxlint --type-aware --tsconfig tsconfig.lint.json --fix", "fix:fmt": "oxfmt \"{src,test,scripts,plugin/src}/**/**.ts\" \"{src,test}/**/**.tsx\"", - "lint": "npx run-s lint:oxlint lint:fmt", + "lint": "npx run-s lint:oxlint lint:fmt lint:side-effects", "lint:oxlint": "sh -c 'OUT=$(OXLINT_TSGOLINT_DANGEROUSLY_SUPPRESS_PROGRAM_DIAGNOSTICS=true oxlint --type-aware --tsconfig tsconfig.lint.json --deny-warnings 2>&1); echo \"$OUT\"; echo \"$OUT\" | grep -qE \"Found 0 warnings and 0 errors\"'", + "lint:side-effects": "node scripts/check-side-effects.js", "lint:fmt": "oxfmt --check \"{src,test,scripts,plugin/src}/**/**.ts\" \"{src,test}/**/**.tsx\"", "api-report:generate": "api-extractor run --local --verbose", "api-report:check": "api-extractor run --verbose" diff --git a/packages/core/scripts/check-side-effects.js b/packages/core/scripts/check-side-effects.js new file mode 100644 index 0000000000..22b107885b --- /dev/null +++ b/packages/core/scripts/check-side-effects.js @@ -0,0 +1,59 @@ +const fs = require('fs'); +const path = require('path'); +const ts = require('typescript'); + +// Guards the `"sideEffects": false` contract in package.json: no module that ships +// in the bundle may run code at import time. A bundler is free to drop such a module +// when its exports are unused, so an import-time side effect would silently vanish +// from a consumer's build. This fails if one is introduced. +// +// Flagged, at module top level only: +// - bare side-effect imports: import './x' / import 'pkg' +// - executed statements run for effect: calls, assignments, IIFEs, global writes +// Declarations are fine, including `const x = f()`: they are module-local and ride +// with the module only when it is kept. `tools/` is excluded (Node-only build tools, +// never bundled into an app). + +const ROOT = path.resolve(__dirname, '..'); +const SRC = path.join(ROOT, 'src', 'js'); +const EXCLUDE_DIRS = [path.join(SRC, 'tools')]; + +function walk(dir, out) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (EXCLUDE_DIRS.some(d => p === d || p.startsWith(d + path.sep))) { + continue; + } + walk(p, out); + } else if (/\.tsx?$/.test(entry.name) && !/\.(test|spec)\.tsx?$/.test(entry.name)) { + out.push(p); + } + } + return out; +} + +const findings = []; +for (const file of walk(SRC, [])) { + const text = fs.readFileSync(file, 'utf8'); + const sourceFile = ts.createSourceFile(file, text, ts.ScriptTarget.ES2018, true, ts.ScriptKind.TSX); + for (const stmt of sourceFile.statements) { + const lineOf = () => sourceFile.getLineAndCharacterOfPosition(stmt.getStart(sourceFile)).line + 1; + if (ts.isImportDeclaration(stmt) && !stmt.importClause) { + findings.push({ file, line: lineOf(), kind: 'bare side-effect import', code: stmt.getText(sourceFile) }); + } else if (ts.isExpressionStatement(stmt)) { + findings.push({ file, line: lineOf(), kind: 'top-level executed statement', code: stmt.getText(sourceFile).slice(0, 100) }); + } + } +} + +const rel = f => path.relative(ROOT, f); +if (findings.length) { + console.error(`\n✖ import-time side effects found (${findings.length}) — these break the "sideEffects": false contract:\n`); + for (const f of findings) { + console.error(` ${rel(f.file)}:${f.line} [${f.kind}] ${f.code.replace(/\n/g, ' ')}`); + } + console.error('\nMove the side effect into a function called by init()/wrap()/an integration factory.\n'); + process.exit(1); +} +console.log('✓ no import-time side effects in src/js (tools/ excluded) — "sideEffects": false is safe'); From aabb0315a0869b96ec07917e87978b06e1a182d2 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 5 Oct 2026 10:21:23 +0200 Subject: [PATCH 2/2] ci(core): Harden side-effect scan against wrapped statements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/core/scripts/check-side-effects.js | 64 +++++++++++++++++---- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/packages/core/scripts/check-side-effects.js b/packages/core/scripts/check-side-effects.js index 22b107885b..9da99af602 100644 --- a/packages/core/scripts/check-side-effects.js +++ b/packages/core/scripts/check-side-effects.js @@ -7,16 +7,56 @@ const ts = require('typescript'); // when its exports are unused, so an import-time side effect would silently vanish // from a consumer's build. This fails if one is introduced. // -// Flagged, at module top level only: -// - bare side-effect imports: import './x' / import 'pkg' -// - executed statements run for effect: calls, assignments, IIFEs, global writes -// Declarations are fine, including `const x = f()`: they are module-local and ride -// with the module only when it is kept. `tools/` is excluded (Node-only build tools, -// never bundled into an app). +// Allowlist approach: only pure declarations are permitted at module top level; +// every other statement is flagged, since anything else can execute at import time +// (an `if`/`for`/`try`/block wrapping a call, a bare expression, an IIFE, a global +// write). A side-effect import (`import 'x'` or `import {} from 'x'`, i.e. one that +// binds nothing) is flagged too. Declarations are fine, including `const x = f()`: +// they are module-local and ride with the module only when it is kept. +// +// Excluded: `tools/` (Node-only build tools, never bundled into an app) and `vendor/` +// (audited third-party code reviewed when it is vendored in). The guard's job is to +// stop first-party SDK code from gaining an import-time side effect. const ROOT = path.resolve(__dirname, '..'); const SRC = path.join(ROOT, 'src', 'js'); -const EXCLUDE_DIRS = [path.join(SRC, 'tools')]; +const EXCLUDE_DIRS = [path.join(SRC, 'tools'), path.join(SRC, 'vendor')]; + +// Top-level statement kinds that are pure declarations (no import-time execution). +const ALLOWED_KINDS = new Set([ + ts.SyntaxKind.FunctionDeclaration, + ts.SyntaxKind.ClassDeclaration, + ts.SyntaxKind.InterfaceDeclaration, + ts.SyntaxKind.TypeAliasDeclaration, + ts.SyntaxKind.EnumDeclaration, + ts.SyntaxKind.ModuleDeclaration, // namespace / declare module + ts.SyntaxKind.VariableStatement, // const/let/var — incl. `const x = f()` + ts.SyntaxKind.ExportDeclaration, // export { ... } / export * from + ts.SyntaxKind.ExportAssignment, // export default ... / export = + ts.SyntaxKind.ImportEqualsDeclaration, + ts.SyntaxKind.EmptyStatement, +]); + +// An import binds nothing (pure side-effect import) when it has no clause, or a clause +// with neither a default name nor namespace nor any named binding. Type-only imports +// are erased at compile time, so they never run. +function isSideEffectImport(node) { + const clause = node.importClause; + if (clause && clause.isTypeOnly) { + return false; + } + if (!clause) { + return true; + } + if (clause.name) { + return false; + } + const bindings = clause.namedBindings; + if (!bindings) { + return true; + } + return ts.isNamedImports(bindings) && bindings.elements.length === 0; +} function walk(dir, out) { for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { @@ -39,9 +79,11 @@ for (const file of walk(SRC, [])) { const sourceFile = ts.createSourceFile(file, text, ts.ScriptTarget.ES2018, true, ts.ScriptKind.TSX); for (const stmt of sourceFile.statements) { const lineOf = () => sourceFile.getLineAndCharacterOfPosition(stmt.getStart(sourceFile)).line + 1; - if (ts.isImportDeclaration(stmt) && !stmt.importClause) { - findings.push({ file, line: lineOf(), kind: 'bare side-effect import', code: stmt.getText(sourceFile) }); - } else if (ts.isExpressionStatement(stmt)) { + if (ts.isImportDeclaration(stmt)) { + if (isSideEffectImport(stmt)) { + findings.push({ file, line: lineOf(), kind: 'side-effect import', code: stmt.getText(sourceFile) }); + } + } else if (!ALLOWED_KINDS.has(stmt.kind)) { findings.push({ file, line: lineOf(), kind: 'top-level executed statement', code: stmt.getText(sourceFile).slice(0, 100) }); } } @@ -56,4 +98,4 @@ if (findings.length) { console.error('\nMove the side effect into a function called by init()/wrap()/an integration factory.\n'); process.exit(1); } -console.log('✓ no import-time side effects in src/js (tools/ excluded) — "sideEffects": false is safe'); +console.log('✓ no import-time side effects in src/js (tools/, vendor/ excluded) — "sideEffects": false is safe');