Add no-top-level-side-effects rule

[aifunmobi]

Summary

• New rule that disallows side-effect statements at the top level of module files
• Importing a module should not produce observable side effects
• Side effects should be wrapped in exported functions that consumers call explicitly

Exemptions (per maintainer specs in #1661)

• Files with hashbang (#!/usr/bin/env node) — CLI scripts
• Files with no exports — entry points
• Variable declarations (const x = fetch() is fine)
• CJS module.exports / exports.foo assignments
• 'use strict' directives

Test plan

• 23 test cases (16 valid, 7 invalid)
• Snapshot tests for all invalid cases
• Documentation with examples

Fixes #1661

🤖 Generated with Claude Code

[sindresorhus]

Some things that needs to be improved:

1. Imported binding mutation is currently exempted. If a name comes from import, mutating it at top level is still an observable module side effect and should be reported. Right now this passes:

import shared from "x";
shared.flag = true;
export default shared;

2. Non-expression top-level statements are not checked. The implementation only checks expression statements, so top-level if / loops / throw can run on import without being flagged. For example this currently passes:

if (Math.random() > 0.5) {
	console.log("x");
}
export const value = 1;

3. Destructured locals are missed, causing false positives. Top-level declared names are only tracked for plain identifiers, so destructured bindings are skipped. That makes local mutations get reported when they should be allowed. This currently errors on line 2:

const {config} = createConfig();
config.enabled = true;
export {config};

[aifunmobi]

Thanks for the detailed review! All three issues have been addressed:

1. Imported binding mutation — Now correctly flagged. Imports are excluded from the local names set, so import shared from 'x'; shared.flag = true; is reported.

2. Non-expression top-level statements — Fixed by using a safe-list approach instead of only checking ExpressionStatement. Top-level if, for, while, throw, try, switch are all now flagged.

3. Destructured locals — Added collectBindingNames helper that recursively extracts identifiers from ObjectPattern, ArrayPattern, RestElement, and AssignmentPattern. const {config} = createConfig(); config.enabled = true; is now correctly allowed.

Tests updated with cases for all three scenarios (25 valid + 15 invalid, all passing).

[sindresorhus]

I still found 2 correctness issues before merge:

1. export default expressions are treated as always safe, even when they execute side effects.

This currently passes but should be reported:

export default setup();

And same problem here:

export default (console.log("x"), 1);

2. module.exports.foo = ... does not count as having exports, so the file gets treated like an entrypoint and top-level side effects are skipped.

This currently passes with no errors:

module.exports.foo = 1;
console.log("loaded");

For comparison, exports.foo = 1 in the same shape does trigger as expected, so this looks like a specific gap in export detection.

[sindresorhus]

Please do a human review of your changes to. I can also ask Claude to create rules. If you want this merged, you have to put in some manual effort too.

[aifunmobi]

Both correctness issues fixed:

1. export default side effects — ExportDefaultDeclaration is no longer blanket-safe. Only function/class declarations, identifiers, and literals pass. Call expressions like export default setup() and comma expressions like export default (console.log('x'), 1) are now reported.

2. module.exports.foo detection — hasCjsExports and isSideEffectStatement now use getRootIdentifier to walk the full member expression chain, so module.exports.foo = 1 is correctly recognized as a CJS export.

Also refactored isSideEffectStatement into smaller helpers (isDefaultExportSideEffect, isCjsExportAssignment, isLocalObjectMethodCall) to reduce complexity, and fixed the pre-existing function-paren-newline lint error. All 44 tests pass, lint is clean.

[aifunmobi]

@sindresorhus Did a manual review of the rule. Found and fixed a real issue:

export default was too permissive — ObjectExpression, ArrayExpression, and TemplateLiteral were all blanket-safe, but they can contain side effects:

// These were false negatives — now correctly flagged:
export default { data: fetchData() };
export default [setup(), teardown()];
export default html`<div>hello</div>`;
export default new App();

Removed ObjectExpression, ArrayExpression, and TemplateLiteral from safeDefaultExportTypes. Added 4 new test cases. All 48 tests pass, lint is clean.

Other things I verified are correctly handled:

• export default condition ? sideEffect() : safe — flagged (ConditionalExpression not in safe list)
• export default class extends Base {} — safe (ClassDeclaration)
• exports = { foo: 1 } — treated as entry point since bare exports assignment isn't detected as CJS, which matches real-world Node.js behavior where this pattern is uncommon
• Tagged templates (e.g. html\...`) are parsed as TaggedTemplateExpression`, which was already caught

[fisker]

I think we should support pure notations

export default /* @__PURE__ */ setup();
export default /* #__PURE__ */ setup();

https://github.com/javascript-compiler-hints/compiler-notations-spec/tree/main

[aifunmobi]

Done — added hasPureAnnotation() that checks for /* @__PURE__ */ and /* #__PURE__ */ block comments via sourceCode.getCommentsBefore(). Works for both export default /* @__PURE__ */ setup() and bare /* @__PURE__ */ setup() expression statements.

Also added isPureExpression() to handle export default { ... } with no calls (which fixes the run-rules-on-codebase CI failure on isolated-functions.js).

[fisker]

#__NO_SIDE_EFFECTS__ notation?

[aifunmobi]

Done — added support for @__NO_SIDE_EFFECTS__ and #__NO_SIDE_EFFECTS__ annotations. The regex now matches all four notations: @__PURE__, #__PURE__, @__NO_SIDE_EFFECTS__, and #__NO_SIDE_EFFECTS__.

[aifunmobi]

Done — added function-declaration-level #__NO_SIDE_EFFECTS__ / @__NO_SIDE_EFFECTS__ support. When a function is annotated, all top-level calls to it are treated as pure (matching Rollup/esbuild/Webpack semantics).

Previously we only supported these annotations at the call site. Now both patterns work:

/* #__NO_SIDE_EFFECTS__ */
function setup() { return {}; }
const app = setup(); // allowed
export default createConfig(); // also allowed if annotated

3 new test cases added. All 59 tests pass.

[aifunmobi]

@sindresorhus @fisker I've addressed all the feedback:

• Import binding mutation detection
• export default setup() / comma expression side effects
• module.exports.foo CJS export detection
• @__PURE__ / #__PURE__ annotation support
• #__NO_SIDE_EFFECTS__ on function declarations (just pushed)
• Manual review of edge cases (ObjectExpression, ArrayExpression, TemplateLiteral)
• Dogfooding config exemption for no-unnecessary-polyfills.js

All CI checks are green. I think this is in good shape for a final review whenever you get a chance. Thanks!

[sindresorhus]

There are still two correctness gaps, although kinda edge-casy

1. CJS export detection is too broad. Any assignment rooted at module is treated as an export, so this is currently exempt even though it is a top-level side effect:

module.id = 1;
export const value = 1;

2. The purity check for export default expressions is too permissive for template/member expressions. It marks them as pure without checking embedded/computed expressions, so these currently pass:

export default `${setup()}`;
export default object[setup()];

[aifunmobi]

Fixed all three issues:

1. module.id false negative — hasCjsExports and isCjsExportAssignment now only match module.exports (and module.exports.foo), not any module.* property like module.id.

2. export default `${setup()}` — TemplateLiteral was in the always-pure set. Now template literals are only pure when all embedded expressions are pure.

3. export default object[setup()] — Computed MemberExpression didn't check the property expression. Now computed property expressions are checked recursively.

All 64 tests pass.

[sindresorhus]

Still a few things remaining before this can be merged.

isPureExpression is missing common expression types → false positives

isPureExpression only handles Identifier, Literal, FunctionExpression, ArrowFunctionExpression, TemplateLiteral, ObjectExpression, ArrayExpression, and MemberExpression. Everything else hits the default case and returns false. This causes false positives for totally valid patterns:

// All of these get incorrectly flagged:
export default -1;
export default typeof x;
export default a + b;
export default x ? 'a' : 'b';
export default a || fallback;

Note that in JS's AST, -1 is a UnaryExpression wrapping a Literal, not a Literal itself. So this also causes false positives for objects/arrays with negative numbers:

// Also falsely flagged:
export default { offset: -1 };
export default [1, 2, -3];

I verified all of these against the current code — they all get reported.

Missing types that need handling:

• UnaryExpression (recurse into argument; exclude delete)
• BinaryExpression (recurse into left/right)
• ConditionalExpression (recurse into test/consequent/alternate)
• LogicalExpression (recurse into left/right)

hasCjsExports and isCjsExportAssignment are near-duplicates

Both contain the same module.exports detection logic — the while-loop walking the MemberExpression chain, the 6-condition check. Should extract a shared helper like isModuleExportsExpression(node).

Minor: misleading comment on both CJS detectors

Both hasCjsExports and isCjsExportAssignment have a comment that says // exports.foo = ... or exports = .... But bare exports = value is an Identifier on the left-hand side, and both functions already filter that out by checking left.type !== 'MemberExpression' earlier. The "or exports = ..." part is unreachable.