From c52916db2bbf084870a252c451f767023b33c025 Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 12:17:39 +0100 Subject: [PATCH 1/8] chore: declare sideEffects: false and guard the claim in CI publint suggested the field. The measurement says it saves a consumer nothing: fixtures bundled under rollup 4.65, vite 7.3, webpack 5.112 and esbuild 0.25, against two tarballs differing only in this field, gave a zero byte delta for all three entry points, minified and not, for import and require consumers alike. splitting: false emits each entry as one self-contained module and sideEffects is module-granularity, so there is nothing for it to drop. The harness detects a delta where one exists: a bare unused import goes from 51,355 bytes to 30. The field therefore goes in because it is true and currently missing, not to save bytes, which makes the guard the substance of this change. Nothing verified the claim before: publint and attw both ignore the field, and every test imports from src/, so the built artifacts' import behaviour was never exercised at all. The guard imports each published entry in a child process with the browser globals, console and the stream writers trapped, and fails on any access. Module-scope reads fail as well as writes, because environment detection at import time breaks the SSR-safety the README claims. Results come back through a file rather than the child's stdout, since a module that logs while evaluating would corrupt them, and that is exactly the module this check exists to catch. The probe self-tests against a generated impure module on every run: a check that cannot fail is worse than no check. --- CLAUDE.md | 2 + package.json | 3 +- scripts/assert-no-import-side-effects.js | 236 +++++++++++++++++++++++ 3 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 scripts/assert-no-import-side-effects.js diff --git a/CLAUDE.md b/CLAUDE.md index 6c198a0..77bef6d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,6 +17,8 @@ - All URL-facing operations convert keys to `snake_case` internally, regardless of the consumer's `keyFormat` setting. - `check:package` runs attw with `--profile node16`, so the `node10` row is still printed but every cell is marked `(ignored)` and it cannot affect the exit code. It is red for `/react` and `/server` because legacy resolution cannot see `exports` subpaths; that is a declared non-target, not a bug to fix. Do not add `typesVersions`: it would green the row for types only. - publint and attw each catch what the other misses. attw exits 0 on a tarball containing no declarations at all, so publint is what catches missing or unpublished files; publint passes a correctly-wired `exports` map whose declarations import chunks the build never emitted, so attw is what catches that. Dropping either silently loses a class of failure. +- `"sideEffects": false` is a claim about the built output that neither publint nor attw checks, and the unit tests only import from `src/`. `scripts/assert-no-import-side-effects.js` is what guards it: it imports every published entry in a child process with the browser globals, `console` and the stream writers trapped. Module-scope *reads* fail too, not only writes, because environment detection at import time breaks the SSR-safety the README claims. It self-tests against a generated impure module on every run; keep that, a probe that cannot fail is worse than none. +- The flag saves a consumer no bytes today. `splitting: false` emits each entry as one self-contained module, and `sideEffects` is module-granularity, so there is nothing for it to drop; it is there to be true, and to pay off if splitting is ever enabled. ## Releases diff --git a/package.json b/package.json index 4eb376c..fc95c49 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ } } }, + "sideEffects": false, "files": [ "dist", "README.md", @@ -50,7 +51,7 @@ "test:watch": "vitest", "coverage": "vitest run --coverage", "type-check": "tsc --noEmit", - "check:package": "npm run build && publint --strict && attw --pack . --profile node16", + "check:package": "npm run build && publint --strict && attw --pack . --profile node16 && node scripts/assert-no-import-side-effects.js", "lint": "oxlint", "lint:fix": "oxlint --fix", "format": "prettier --write \"src/**/*.{ts,tsx}\" \"__tests__/**/*.{ts,tsx}\"", diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js new file mode 100644 index 0000000..b21abc7 --- /dev/null +++ b/scripts/assert-no-import-side-effects.js @@ -0,0 +1,236 @@ +#!/usr/bin/env node + +/** + * Fails if importing a published entry point evaluates observable code. + * + * `sideEffects: false` lets a consumer's bundler drop an unused module without + * evaluating it. Neither publint nor attw checks that, and the unit tests import + * from src/, so nothing else exercises the built artifacts' import behaviour. + * + * Usage: node scripts/assert-no-import-side-effects.js + */ + +import { spawnSync } from 'child_process' +import { createRequire } from 'module' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs' +import { fileURLToPath, pathToFileURL } from 'url' +import { dirname, join, resolve } from 'path' +import { tmpdir } from 'os' + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const root = resolve(__dirname, '..') +const selfPath = fileURLToPath(import.meta.url) + +/** + * Read or assignment of any of these during module evaluation fails the check. + * Reads are included deliberately: a module-scope read means environment + * detection at import time, which breaks the SSR-safety the README claims. + */ +const TRAPPED_GLOBALS = [ + 'window', + 'document', + 'sessionStorage', + 'localStorage', + 'navigator', + 'location', + 'MutationObserver', + 'XMLHttpRequest', + 'fetch', + 'self', + 'top', + 'parent', +] + +/** Imported before the traps go up, so their own evaluation is not attributed here. */ +const EXTERNALS = ['react', 'react/jsx-runtime'] + +function collectEntries(exportsMap) { + const found = new Set() + const walk = (node) => { + if (typeof node === 'string') { + if (node.endsWith('.js') || node.endsWith('.cjs')) found.add(node) + return + } + if (node && typeof node === 'object') { + for (const value of Object.values(node)) walk(value) + } + } + walk(exportsMap) + return [...found].sort() +} + +async function runProbe(target, format, outFile) { + const require = createRequire(selfPath) + + for (const dep of EXTERNALS) { + try { + require(dep) + } catch { + // Not installed in this context; nothing to warm. + } + } + + const accesses = [] + const unprotected = [] + + for (const name of TRAPPED_GLOBALS) { + try { + Object.defineProperty(globalThis, name, { + configurable: true, + get() { + accesses.push(`read ${name}`) + return undefined + }, + set() { + accesses.push(`assign ${name}`) + }, + }) + } catch { + unprotected.push(name) + } + } + + for (const method of Object.keys(console)) { + if (typeof console[method] !== 'function') continue + console[method] = () => accesses.push(`console.${method}()`) + } + + for (const stream of ['stdout', 'stderr']) { + process[stream].write = () => { + accesses.push(`process.${stream}.write()`) + return true + } + } + + let failure = null + try { + if (format === 'cjs') { + require(target) + } else { + await import(pathToFileURL(target).href) + } + } catch (error) { + failure = error instanceof Error ? error.message : String(error) + } + + writeFileSync(outFile, JSON.stringify({ accesses, unprotected, failure })) +} + +/** + * Results come back through a file, not stdout: a module that logs or writes to + * a stream during evaluation would otherwise corrupt them, which is exactly the + * module this check exists to catch. + */ +function probe(target, format) { + const dir = mkdtempSync(join(tmpdir(), 'utm-probe-')) + const outFile = join(dir, 'result.json') + try { + const result = spawnSync(process.execPath, [selfPath, '--probe', target, format, outFile], { + encoding: 'utf-8', + cwd: root, + }) + + try { + return JSON.parse(readFileSync(outFile, 'utf-8')) + } catch { + const detail = result.stderr.trim() || result.stdout.trim() || 'no output' + return { + accesses: [], + unprotected: [], + failure: `probe process exited ${result.status} without a result: ${detail}`, + } + } + } finally { + rmSync(dir, { recursive: true, force: true }) + } +} + +/** + * The check is worthless if the probe cannot report a side effect, so prove on + * every run that it does. Returns the reason it is untrustworthy, or null. + */ +function selfTestFailure() { + const dir = mkdtempSync(join(tmpdir(), 'utm-side-effects-')) + try { + const body = + "console.log('impure')\nprocess.stdout.write('impure')\nconst probed = sessionStorage\n" + const cases = [ + [join(dir, 'impure.mjs'), `${body}export default probed\n`, 'esm'], + [join(dir, 'impure.cjs'), `${body}module.exports = probed\n`, 'cjs'], + ] + + for (const [file, source, format] of cases) { + writeFileSync(file, source) + const { accesses, failure } = probe(file, format) + if (failure) return `probing a known-impure ${format} module errored: ${failure}` + if (!accesses.includes('console.log()')) { + return `probe missed a console.log() in a known-impure ${format} module` + } + if (!accesses.includes('process.stdout.write()')) { + return `probe missed a process.stdout.write() in a known-impure ${format} module` + } + if (!accesses.includes('read sessionStorage')) { + return `probe missed a sessionStorage read in a known-impure ${format} module` + } + } + return null + } finally { + rmSync(dir, { recursive: true, force: true }) + } +} + +function main() { + const pkg = JSON.parse(readFileSync(join(root, 'package.json'), 'utf-8')) + const entries = collectEntries(pkg.exports) + + if (entries.length === 0) { + console.error('No .js or .cjs entry points found in package.json exports.') + process.exit(1) + } + + console.log(`Checking ${entries.length} entry points for import-time side effects...`) + + const selfTest = selfTestFailure() + if (selfTest) { + console.error(`Probe self-test failed, so this check cannot be trusted: ${selfTest}`) + process.exit(1) + } + + const problems = [] + for (const entry of entries) { + const target = join(root, entry) + const format = entry.endsWith('.cjs') ? 'cjs' : 'esm' + const { accesses, unprotected, failure } = probe(target, format) + + if (failure) { + problems.push(`${entry}: could not be imported (${failure}). Run \`npm run build\` first.`) + continue + } + if (unprotected.length > 0) { + problems.push(`${entry}: could not trap ${unprotected.join(', ')}, so it went unchecked.`) + } + if (accesses.length > 0) { + problems.push(`${entry}: evaluated on import — ${[...new Set(accesses)].join(', ')}`) + } + } + + if (problems.length > 0) { + console.error('\nImport-time side effects:') + problems.forEach((problem, index) => console.error(`${index + 1}. ${problem}`)) + console.error('\n"sideEffects": false in package.json tells bundlers this cannot happen.') + process.exit(1) + } + + console.log('No import-time side effects.') +} + +const probeIndex = process.argv.indexOf('--probe') +if (probeIndex === -1) { + main() +} else { + await runProbe( + process.argv[probeIndex + 1], + process.argv[probeIndex + 2], + process.argv[probeIndex + 3], + ) +} From f5d29d5e99609360242e9ad0beff070d3541d613 Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 12:24:22 +0100 Subject: [PATCH 2/8] fix: fail on handles left behind at import instead of hanging on them A module that started a timer during evaluation kept the probe's child process alive, and spawnSync had no timeout, so the parent blocked until the CI job's own limit - six hours by default, with publish.yml's gate stuck behind it. A side effect this check exists to catch could stall the pipeline rather than fail it. Exiting the child after it writes its result would have cured the hang by turning that case into a silent pass, which is worse. So the child now compares process.getActiveResourcesInfo() either side of the import and reports anything new, then exits. That covers the class generically rather than by enumerating APIs: a timer reports as Timeout, a listening socket as TCPServerWrap, without either being trapped by name. spawnSync keeps a 30s timeout as a backstop for a handle the counter cannot see. The self-test gains timer fixtures in both module formats, so the detector cannot quietly stop working. Also widens the import-failure hint, which blamed a missing build for what is equally often a missing dependency. --- scripts/assert-no-import-side-effects.js | 52 +++++++++++++++++------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js index b21abc7..c96a329 100644 --- a/scripts/assert-no-import-side-effects.js +++ b/scripts/assert-no-import-side-effects.js @@ -44,6 +44,12 @@ const TRAPPED_GLOBALS = [ /** Imported before the traps go up, so their own evaluation is not attributed here. */ const EXTERNALS = ['react', 'react/jsx-runtime'] +function countByType(resources) { + const counts = {} + for (const type of resources) counts[type] = (counts[type] ?? 0) + 1 + return counts +} + function collectEntries(exportsMap) { const found = new Set() const walk = (node) => { @@ -102,6 +108,8 @@ async function runProbe(target, format, outFile) { } } + const resourcesBefore = countByType(process.getActiveResourcesInfo()) + let failure = null try { if (format === 'cjs') { @@ -113,7 +121,17 @@ async function runProbe(target, format, outFile) { failure = error instanceof Error ? error.message : String(error) } + const resourcesAfter = countByType(process.getActiveResourcesInfo()) + for (const [type, count] of Object.entries(resourcesAfter)) { + const added = count - (resourcesBefore[type] ?? 0) + if (added > 0) accesses.push(`left ${added} active ${type} handle(s) behind`) + } + writeFileSync(outFile, JSON.stringify({ accesses, unprotected, failure })) + + // A timer, socket or listener started during evaluation would otherwise keep + // this process alive and stall the parent instead of failing it. + process.exit(0) } /** @@ -128,6 +146,7 @@ function probe(target, format) { const result = spawnSync(process.execPath, [selfPath, '--probe', target, format, outFile], { encoding: 'utf-8', cwd: root, + timeout: 30_000, }) try { @@ -152,25 +171,28 @@ function probe(target, format) { function selfTestFailure() { const dir = mkdtempSync(join(tmpdir(), 'utm-side-effects-')) try { - const body = + const accessBody = "console.log('impure')\nprocess.stdout.write('impure')\nconst probed = sessionStorage\n" + const accessExpectations = ['console.log()', 'process.stdout.write()', 'read sessionStorage'] + const timerBody = 'const handle = setInterval(() => {}, 1000)\n' + const timerExpectations = ['active Timeout handle'] + const cases = [ - [join(dir, 'impure.mjs'), `${body}export default probed\n`, 'esm'], - [join(dir, 'impure.cjs'), `${body}module.exports = probed\n`, 'cjs'], + ['impure.mjs', `${accessBody}export default probed\n`, 'esm', accessExpectations], + ['impure.cjs', `${accessBody}module.exports = probed\n`, 'cjs', accessExpectations], + ['timer.mjs', `${timerBody}export default handle\n`, 'esm', timerExpectations], + ['timer.cjs', `${timerBody}module.exports = handle\n`, 'cjs', timerExpectations], ] - for (const [file, source, format] of cases) { + for (const [name, source, format, expectations] of cases) { + const file = join(dir, name) writeFileSync(file, source) const { accesses, failure } = probe(file, format) - if (failure) return `probing a known-impure ${format} module errored: ${failure}` - if (!accesses.includes('console.log()')) { - return `probe missed a console.log() in a known-impure ${format} module` - } - if (!accesses.includes('process.stdout.write()')) { - return `probe missed a process.stdout.write() in a known-impure ${format} module` - } - if (!accesses.includes('read sessionStorage')) { - return `probe missed a sessionStorage read in a known-impure ${format} module` + if (failure) return `probing known-impure ${name} errored: ${failure}` + for (const expected of expectations) { + if (!accesses.some((access) => access.includes(expected))) { + return `probe missed ${expected} in known-impure ${name}` + } } } return null @@ -203,7 +225,9 @@ function main() { const { accesses, unprotected, failure } = probe(target, format) if (failure) { - problems.push(`${entry}: could not be imported (${failure}). Run \`npm run build\` first.`) + problems.push( + `${entry}: could not be imported (${failure}). Run \`npm ci\` and \`npm run build\` first.`, + ) continue } if (unprotected.length > 0) { From 665da1f6c19717796a48f4a34c7625c1abe17200 Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 12:25:43 +0100 Subject: [PATCH 3/8] docs: give the right reason for failing module-scope reads The reason given was false. `typeof window !== 'undefined'` under the probe evaluates to false and never throws, and a module caching that on a server still returns the empty values the README promises, so it does not break SSR-safety. What throws on a server is a module-scope property read such as `window.location.search`, and the trap sees both as "read window". So the rule stands on the fact that the trap cannot tell those apart, which makes failing every read the conservative choice, and on the house invariant: all 13 guard sites in src/ sit inside function bodies, and __tests__/setup.ts re-stubs those globals per test, so a cached module-level read would make the suite order-dependent. A wrong reason is what invites someone to conclude the rule itself is wrong. Also drops the clauses that only restated the script header and the commit message, and corrects "saves no bytes", which the measurement contradicts for a consumer importing nothing at all: that case drops 51,355 bytes to 30. --- CLAUDE.md | 4 ++-- scripts/assert-no-import-side-effects.js | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 77bef6d..c6765d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,8 +17,8 @@ - All URL-facing operations convert keys to `snake_case` internally, regardless of the consumer's `keyFormat` setting. - `check:package` runs attw with `--profile node16`, so the `node10` row is still printed but every cell is marked `(ignored)` and it cannot affect the exit code. It is red for `/react` and `/server` because legacy resolution cannot see `exports` subpaths; that is a declared non-target, not a bug to fix. Do not add `typesVersions`: it would green the row for types only. - publint and attw each catch what the other misses. attw exits 0 on a tarball containing no declarations at all, so publint is what catches missing or unpublished files; publint passes a correctly-wired `exports` map whose declarations import chunks the build never emitted, so attw is what catches that. Dropping either silently loses a class of failure. -- `"sideEffects": false` is a claim about the built output that neither publint nor attw checks, and the unit tests only import from `src/`. `scripts/assert-no-import-side-effects.js` is what guards it: it imports every published entry in a child process with the browser globals, `console` and the stream writers trapped. Module-scope *reads* fail too, not only writes, because environment detection at import time breaks the SSR-safety the README claims. It self-tests against a generated impure module on every run; keep that, a probe that cannot fail is worse than none. -- The flag saves a consumer no bytes today. `splitting: false` emits each entry as one self-contained module, and `sideEffects` is module-granularity, so there is nothing for it to drop; it is there to be true, and to pay off if splitting is ever enabled. +- `scripts/assert-no-import-side-effects.js` guards `"sideEffects": false`, which publint reports on but neither publint nor attw verifies. Module-scope reads of the browser globals fail it, not only writes: `window.location` there throws on a server, and the trap cannot tell that from a harmless `typeof window`. Detect the environment at call time, as all 13 guard sites in `src/` do; `__tests__/setup.ts` re-stubs those globals per test, so a cached module-level read also makes the suite order-dependent. +- The flag saves no bytes for any consumer that uses an export. `splitting: false` emits each entry as one self-contained JS module and `sideEffects` is module-granularity, so there is nothing for it to drop. ## Releases diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js index c96a329..f6028a0 100644 --- a/scripts/assert-no-import-side-effects.js +++ b/scripts/assert-no-import-side-effects.js @@ -23,8 +23,9 @@ const selfPath = fileURLToPath(import.meta.url) /** * Read or assignment of any of these during module evaluation fails the check. - * Reads are included deliberately: a module-scope read means environment - * detection at import time, which breaks the SSR-safety the README claims. + * Reads fail too, not only writes: `window.location` at module scope throws on a + * server, and this trap cannot tell that from a harmless `typeof window`. Detect + * the environment at call time, as every site in src/ does. */ const TRAPPED_GLOBALS = [ 'window', From 5057912c5cec2dc972fd7b25eae3d755a94cc213 Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 12:33:59 +0100 Subject: [PATCH 4/8] fix: detect the import-time side effects the trap set could not see The guard passed, at exit 0, a built entry that mutated process.env, patched Array.prototype, added a globalThis property and registered a process exit listener. Trapping twelve browser globals plus console and the stream writers covers what this library touches today and nothing else, so it licensed sideEffects: false for a module doing observable work. No enumeration closes that, because the set of things a module can mutate is open. So the probe now snapshots the environment either side of the import - globalThis own properties, process.env entries, the prototypes of Object, Array, String, Number, Function, Promise and Date, and the active resource counts - and reports anything that changed. One pass covers globalThis writes, Object.defineProperty on globalThis, prototype patching and env mutation together. Named traps are added only where a diff cannot see the call at all: the schedulers and the process hooks. Two channels close the remaining gaps. Any child stdout is now itself a side effect, since results travel by file and only the module under test can write there - which catches fs.writeSync(1, ...) bypassing the stream trap. And an entry importing a node builtin fails outright: synchronous work like fs.writeFileSync leaves nothing to observe, so the bundle shape is the only place to rule it out. No entry imports one today. A failing import no longer discards what was recorded before it failed, and the stale-build hint is given only for a module-resolution error, which is what it actually diagnoses. collectEntries now takes every ./ target that is not a declaration or a map and fails on an extension it cannot load, instead of silently checking a subset: an exports map using .mjs for the ESM entry previously left the entry a bundler tree-shakes unchecked while reporting success. The self-test grew to one fixture per detector, so a detector that stops working fails the run rather than the claim. --- CLAUDE.md | 4 +- scripts/assert-no-import-side-effects.js | 221 ++++++++++++++++++----- 2 files changed, 174 insertions(+), 51 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c6765d0..6108e72 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,8 +17,8 @@ - All URL-facing operations convert keys to `snake_case` internally, regardless of the consumer's `keyFormat` setting. - `check:package` runs attw with `--profile node16`, so the `node10` row is still printed but every cell is marked `(ignored)` and it cannot affect the exit code. It is red for `/react` and `/server` because legacy resolution cannot see `exports` subpaths; that is a declared non-target, not a bug to fix. Do not add `typesVersions`: it would green the row for types only. - publint and attw each catch what the other misses. attw exits 0 on a tarball containing no declarations at all, so publint is what catches missing or unpublished files; publint passes a correctly-wired `exports` map whose declarations import chunks the build never emitted, so attw is what catches that. Dropping either silently loses a class of failure. -- `scripts/assert-no-import-side-effects.js` guards `"sideEffects": false`, which publint reports on but neither publint nor attw verifies. Module-scope reads of the browser globals fail it, not only writes: `window.location` there throws on a server, and the trap cannot tell that from a harmless `typeof window`. Detect the environment at call time, as all 13 guard sites in `src/` do; `__tests__/setup.ts` re-stubs those globals per test, so a cached module-level read also makes the suite order-dependent. -- The flag saves no bytes for any consumer that uses an export. `splitting: false` emits each entry as one self-contained JS module and `sideEffects` is module-granularity, so there is nothing for it to drop. +- `scripts/assert-no-import-side-effects.js` guards `"sideEffects": false`, which publint reports on but neither publint nor attw verifies. Module-scope reads of the browser globals fail it, not only writes: `window.location` there throws on a server, and the trap cannot tell that from a harmless `typeof window`. Detect the environment at call time, as all 13 guard sites in `src/` do; `__tests__/setup.ts` re-stubs those globals per test, so a cached module-level read also makes the suite order-dependent. It also fails any entry that imports a node builtin, since it cannot see what such code does at import; no entry needs one today. +- The flag saves a consumer bytes only when none of an entry's exports are used. `sideEffects` is module-granularity and `splitting: false` emits each entry as a single JS module, so there is no partial drop: measured against rollup, vite, webpack and esbuild, every consumer that uses an export saw a zero-byte delta. ## Releases diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js index f6028a0..3eca9be 100644 --- a/scripts/assert-no-import-side-effects.js +++ b/scripts/assert-no-import-side-effects.js @@ -4,8 +4,16 @@ * Fails if importing a published entry point evaluates observable code. * * `sideEffects: false` lets a consumer's bundler drop an unused module without - * evaluating it. Neither publint nor attw checks that, and the unit tests import - * from src/, so nothing else exercises the built artifacts' import behaviour. + * evaluating it. publint reports the field's absence but neither publint nor attw + * verifies the claim, and the unit tests import from src/, so nothing else + * exercises the built artifacts' import behaviour. + * + * Detection is deliberately belt-and-braces, because no single mechanism covers + * the ways module evaluation can be observable: named traps for the globals and + * schedulers a refactor would plausibly reach for, and a before/after diff of the + * environment for everything else. What is NOT detected is synchronous + * filesystem work (`fs.writeFileSync` leaves no trace); the no-builtin-imports + * assertion below covers that class at the bundle level instead. * * Usage: node scripts/assert-no-import-side-effects.js */ @@ -42,20 +50,41 @@ const TRAPPED_GLOBALS = [ 'parent', ] -/** Imported before the traps go up, so their own evaluation is not attributed here. */ +/** Calling any of these during evaluation is itself the side effect. */ +const TRAPPED_SCHEDULERS = ['setTimeout', 'setInterval', 'setImmediate', 'queueMicrotask'] +const TRAPPED_PROCESS_METHODS = ['nextTick', 'on', 'once', 'addListener', 'prependListener'] + +/** Patching any of these is a side effect the environment diff reports by name. */ +const WATCHED_PROTOTYPES = ['Object', 'Array', 'String', 'Number', 'Function', 'Promise', 'Date'] + +/** + * Imported before the traps go up, so their own evaluation is not attributed + * here. Must stay in step with `external` in tsup.config.ts: an external missing + * from this list has its side effects blamed on our entry. + */ const EXTERNALS = ['react', 'react/jsx-runtime'] -function countByType(resources) { +/** Extensions this script knows how to load; anything else must not be skipped silently. */ +const LOADABLE = ['.js', '.cjs', '.mjs'] +const NOT_RUNTIME = /\.(d\.ts|d\.cts|d\.mts|map)$/ + +const BUILTIN_IMPORT = new RegExp( + String.raw`(?:from|require\()\s*['"](?:node:)?` + + '(fs|http|https|net|path|os|child_process|crypto|stream|zlib|dns|tls|worker_threads|vm|module)' + + String.raw`['"]`, +) + +function countByType(values) { const counts = {} - for (const type of resources) counts[type] = (counts[type] ?? 0) + 1 + for (const value of values) counts[value] = (counts[value] ?? 0) + 1 return counts } function collectEntries(exportsMap) { - const found = new Set() + const paths = new Set() const walk = (node) => { if (typeof node === 'string') { - if (node.endsWith('.js') || node.endsWith('.cjs')) found.add(node) + if (node.startsWith('./')) paths.add(node) return } if (node && typeof node === 'object') { @@ -63,7 +92,45 @@ function collectEntries(exportsMap) { } } walk(exportsMap) - return [...found].sort() + + const entries = [] + const unknown = [] + for (const path of [...paths].sort()) { + if (NOT_RUNTIME.test(path)) continue + if (LOADABLE.some((extension) => path.endsWith(extension))) entries.push(path) + else unknown.push(path) + } + return { entries, unknown } +} + +/** Snapshot taken after the traps are installed, so the traps are not reported as changes. */ +function snapshotEnvironment() { + return { + globals: Object.getOwnPropertyNames(globalThis), + env: Object.entries(process.env).map(([key, value]) => `${key}=${value}`), + prototypes: WATCHED_PROTOTYPES.flatMap((name) => + Object.getOwnPropertyNames(globalThis[name].prototype).map((key) => `${name}.${key}`), + ), + resources: countByType(process.getActiveResourcesInfo()), + } +} + +function diffEnvironment(before, after) { + const changes = [] + const added = (label, from, to) => { + const known = new Set(from) + for (const value of to) if (!known.has(value)) changes.push(`${label} ${value}`) + } + + added('added global', before.globals, after.globals) + added('changed process.env', before.env, after.env) + added('patched', before.prototypes, after.prototypes) + + for (const [type, count] of Object.entries(after.resources)) { + const extra = count - (before.resources[type] ?? 0) + if (extra > 0) changes.push(`left ${extra} active ${type} handle(s) behind`) + } + return changes } async function runProbe(target, format, outFile) { @@ -73,7 +140,7 @@ async function runProbe(target, format, outFile) { try { require(dep) } catch { - // Not installed in this context; nothing to warm. + // Not resolvable here; a real import failure is reported below instead. } } @@ -97,6 +164,20 @@ async function runProbe(target, format, outFile) { } } + for (const name of TRAPPED_SCHEDULERS) { + globalThis[name] = () => { + accesses.push(`${name}()`) + return undefined + } + } + + for (const name of TRAPPED_PROCESS_METHODS) { + process[name] = () => { + accesses.push(`process.${name}()`) + return process + } + } + for (const method of Object.keys(console)) { if (typeof console[method] !== 'function') continue console[method] = () => accesses.push(`console.${method}()`) @@ -109,9 +190,10 @@ async function runProbe(target, format, outFile) { } } - const resourcesBefore = countByType(process.getActiveResourcesInfo()) + const before = snapshotEnvironment() let failure = null + let failureCode = null try { if (format === 'cjs') { require(target) @@ -120,15 +202,12 @@ async function runProbe(target, format, outFile) { } } catch (error) { failure = error instanceof Error ? error.message : String(error) + failureCode = error?.code ?? null } - const resourcesAfter = countByType(process.getActiveResourcesInfo()) - for (const [type, count] of Object.entries(resourcesAfter)) { - const added = count - (resourcesBefore[type] ?? 0) - if (added > 0) accesses.push(`left ${added} active ${type} handle(s) behind`) - } + accesses.push(...diffEnvironment(before, snapshotEnvironment())) - writeFileSync(outFile, JSON.stringify({ accesses, unprotected, failure })) + writeFileSync(outFile, JSON.stringify({ accesses, unprotected, failure, failureCode })) // A timer, socket or listener started during evaluation would otherwise keep // this process alive and stall the parent instead of failing it. @@ -136,9 +215,10 @@ async function runProbe(target, format, outFile) { } /** - * Results come back through a file, not stdout: a module that logs or writes to - * a stream during evaluation would otherwise corrupt them, which is exactly the - * module this check exists to catch. + * Results come back through a file, not stdout: a module that logs or writes to a + * stream during evaluation would otherwise corrupt them, which is exactly the + * module this check exists to catch. That also makes any child stdout a side + * effect in itself, since it can only have come from the module under test. */ function probe(target, format) { const dir = mkdtempSync(join(tmpdir(), 'utm-probe-')) @@ -150,16 +230,28 @@ function probe(target, format) { timeout: 30_000, }) + let parsed try { - return JSON.parse(readFileSync(outFile, 'utf-8')) + parsed = JSON.parse(readFileSync(outFile, 'utf-8')) } catch { - const detail = result.stderr.trim() || result.stdout.trim() || 'no output' + const detail = result.stderr?.trim() || result.stdout?.trim() || 'no output' return { accesses: [], unprotected: [], - failure: `probe process exited ${result.status} without a result: ${detail}`, + failure: `probe exited ${result.status}${result.signal ? ` (${result.signal})` : ''} without a result: ${detail}`, + failureCode: null, } } + + if (result.stdout) { + parsed.accesses.push(`wrote ${result.stdout.length} byte(s) straight to stdout`) + } + if (result.signal) { + parsed.failure = [parsed.failure, `probe killed by ${result.signal}`] + .filter(Boolean) + .join('; ') + } + return parsed } finally { rmSync(dir, { recursive: true, force: true }) } @@ -167,32 +259,41 @@ function probe(target, format) { /** * The check is worthless if the probe cannot report a side effect, so prove on - * every run that it does. Returns the reason it is untrustworthy, or null. + * every run that each detector fires. Returns the reason it is untrustworthy, or + * null. Add a case here whenever a detector is added. */ function selfTestFailure() { const dir = mkdtempSync(join(tmpdir(), 'utm-side-effects-')) try { - const accessBody = - "console.log('impure')\nprocess.stdout.write('impure')\nconst probed = sessionStorage\n" - const accessExpectations = ['console.log()', 'process.stdout.write()', 'read sessionStorage'] - const timerBody = 'const handle = setInterval(() => {}, 1000)\n' - const timerExpectations = ['active Timeout handle'] - const cases = [ - ['impure.mjs', `${accessBody}export default probed\n`, 'esm', accessExpectations], - ['impure.cjs', `${accessBody}module.exports = probed\n`, 'cjs', accessExpectations], - ['timer.mjs', `${timerBody}export default handle\n`, 'esm', timerExpectations], - ['timer.cjs', `${timerBody}module.exports = handle\n`, 'cjs', timerExpectations], + ["console.log('x')\n", ['console.log()']], + ["process.stdout.write('x')\n", ['process.stdout.write()']], + ['const read = sessionStorage\n', ['read sessionStorage']], + ['globalThis.window = {}\n', ['assign window']], + ['setInterval(() => {}, 1000)\n', ['setInterval()']], + ['queueMicrotask(() => {})\n', ['queueMicrotask()']], + ["process.on('exit', () => {})\n", ['process.on()']], + ['globalThis.__utmProbeMarker = 1\n', ['added global __utmProbeMarker']], + ["process.env.UTM_PROBE_MARKER = '1'\n", ['changed process.env UTM_PROBE_MARKER=1']], + ['Array.prototype.__utmProbeMarker = 1\n', ['patched Array.__utmProbeMarker']], + ["require('fs').writeSync(1, 'x')\n", ['straight to stdout']], ] - for (const [name, source, format, expectations] of cases) { - const file = join(dir, name) - writeFileSync(file, source) - const { accesses, failure } = probe(file, format) - if (failure) return `probing known-impure ${name} errored: ${failure}` + let index = 0 + for (const [body, expectations] of cases) { + index += 1 + // Each case runs as CJS so `require` is available to the stdout-bypass case. + const file = join(dir, `impure-${index}.cjs`) + writeFileSync(file, body) + const { accesses, unprotected, failure } = probe(file, 'cjs') + + if (failure) return `probing known-impure case ${index} errored: ${failure}` + if (unprotected.length > 0) { + return `could not trap ${unprotected.join(', ')}, so the check is incomplete` + } for (const expected of expectations) { if (!accesses.some((access) => access.includes(expected))) { - return `probe missed ${expected} in known-impure ${name}` + return `probe missed "${expected}" in known-impure case ${index} (saw: ${accesses.join(', ') || 'nothing'})` } } } @@ -204,10 +305,19 @@ function selfTestFailure() { function main() { const pkg = JSON.parse(readFileSync(join(root, 'package.json'), 'utf-8')) - const entries = collectEntries(pkg.exports) - + const { entries, unknown } = collectEntries(pkg.exports) + + if (unknown.length > 0) { + console.error( + `Cannot probe these entry points, so they would go unchecked: ${unknown.join(', ')}`, + ) + console.error( + `Teach LOADABLE in ${'scripts/assert-no-import-side-effects.js'} how to load them.`, + ) + process.exit(1) + } if (entries.length === 0) { - console.error('No .js or .cjs entry points found in package.json exports.') + console.error('No loadable entry points found in package.json exports.') process.exit(1) } @@ -223,20 +333,33 @@ function main() { for (const entry of entries) { const target = join(root, entry) const format = entry.endsWith('.cjs') ? 'cjs' : 'esm' - const { accesses, unprotected, failure } = probe(target, format) + const { accesses, unprotected, failure, failureCode } = probe(target, format) - if (failure) { - problems.push( - `${entry}: could not be imported (${failure}). Run \`npm ci\` and \`npm run build\` first.`, - ) - continue - } if (unprotected.length > 0) { problems.push(`${entry}: could not trap ${unprotected.join(', ')}, so it went unchecked.`) } if (accesses.length > 0) { problems.push(`${entry}: evaluated on import — ${[...new Set(accesses)].join(', ')}`) } + if (failure) { + const missing = failureCode === 'ERR_MODULE_NOT_FOUND' || failureCode === 'MODULE_NOT_FOUND' + const hint = missing ? ' Run `npm ci` and `npm run build` first.' : '' + problems.push(`${entry}: could not be imported (${failure}).${hint}`) + } + + const source = (() => { + try { + return readFileSync(target, 'utf-8') + } catch { + return '' + } + })() + const builtin = source.match(BUILTIN_IMPORT) + if (builtin) { + problems.push( + `${entry}: imports the node builtin "${builtin[1]}", whose calls this check cannot see.`, + ) + } } if (problems.length > 0) { From 20ab332df30638d59e8d258c64da54580e83aba6 Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 12:42:02 +0100 Subject: [PATCH 5/8] fix: stop NODE_DEBUG=module failing a clean build The probe replaces process.stderr.write before importing, so Node's own module-loader debug lines went through the trap and were reported as the package's side effects: with NODE_DEBUG=module set, four of six entries failed on an unmodified build, blaming the package for the debugger's output. A guard that fails on a clean tree gets switched off by whoever it blocks, which costs more than the class it detects. The child's env drops NODE_DEBUG. A genuine process.stderr.write at import is still caught with NODE_DEBUG=module set, so the detector is unchanged; only Node's own diagnostics stop being attributed to the package. --- scripts/assert-no-import-side-effects.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js index 3eca9be..9f5e2fa 100644 --- a/scripts/assert-no-import-side-effects.js +++ b/scripts/assert-no-import-side-effects.js @@ -223,11 +223,18 @@ async function runProbe(target, format, outFile) { function probe(target, format) { const dir = mkdtempSync(join(tmpdir(), 'utm-probe-')) const outFile = join(dir, 'result.json') + + // NODE_DEBUG=module makes Node log through the trapped stderr writer, which + // would be reported against the package rather than against the debugger. + const env = { ...process.env } + delete env.NODE_DEBUG + try { const result = spawnSync(process.execPath, [selfPath, '--probe', target, format, outFile], { encoding: 'utf-8', cwd: root, timeout: 30_000, + env, }) let parsed From dbb9645e360a5fee4631e12c60103a55c1a956a3 Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 12:52:43 +0100 Subject: [PATCH 6/8] fix: compare the environment by value, so replacement and deletion are seen The diff compared property names, so only additions showed up. A built entry that reassigned Array.prototype.includes, replaced String.prototype.trim and globalThis.structuredClone, deleted Array.prototype.at and called process.chdir passed at exit 0 - verified, with a marker proving those lines ran. Replacing an existing method is the common polyfill shape, so the rewrite closed the rarer half of the class it was aimed at. Globals and prototypes now record an identity token per key, taken from the property descriptor so an accessor's getter is never invoked - the browser traps are accessors, and reading them would report a side effect against the module under test. The diff is symmetric, so addition, replacement and removal all surface, and Object.isFrozen is recorded alongside. process.cwd and process.title are snapshotted too. Adding a negative control to the self-test immediately earned it: a module with no side effects at all was reported as having three. Comparing with === made the NaN global read as replaced on every run, and reading descriptors materialises Node's undici dispatcher, which registers symbol-keyed globals with an incrementing counter, so each snapshot saw new ones. Object.is fixes the first; globals are now compared over string keys only, which costs the detection of a symbol-keyed global and is recorded as a known gap. The builtin check now resolves specifiers against module.builtinModules rather than a hand-written list, and anchors on import() as well, so node:fs/promises and dynamic imports are no longer missed. collectEntries skips wildcard subpaths and package.json, which would otherwise abort the run as unloadable. Scheduler stubs return a Timeout-shaped handle so a module that unrefs one is not also reported as unimportable, and a spawn error now reaches the message. --- CLAUDE.md | 2 +- scripts/assert-no-import-side-effects.js | 172 ++++++++++++++++------- 2 files changed, 120 insertions(+), 54 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6108e72..1a57813 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ - `check:package` runs attw with `--profile node16`, so the `node10` row is still printed but every cell is marked `(ignored)` and it cannot affect the exit code. It is red for `/react` and `/server` because legacy resolution cannot see `exports` subpaths; that is a declared non-target, not a bug to fix. Do not add `typesVersions`: it would green the row for types only. - publint and attw each catch what the other misses. attw exits 0 on a tarball containing no declarations at all, so publint is what catches missing or unpublished files; publint passes a correctly-wired `exports` map whose declarations import chunks the build never emitted, so attw is what catches that. Dropping either silently loses a class of failure. - `scripts/assert-no-import-side-effects.js` guards `"sideEffects": false`, which publint reports on but neither publint nor attw verifies. Module-scope reads of the browser globals fail it, not only writes: `window.location` there throws on a server, and the trap cannot tell that from a harmless `typeof window`. Detect the environment at call time, as all 13 guard sites in `src/` do; `__tests__/setup.ts` re-stubs those globals per test, so a cached module-level read also makes the suite order-dependent. It also fails any entry that imports a node builtin, since it cannot see what such code does at import; no entry needs one today. -- The flag saves a consumer bytes only when none of an entry's exports are used. `sideEffects` is module-granularity and `splitting: false` emits each entry as a single JS module, so there is no partial drop: measured against rollup, vite, webpack and esbuild, every consumer that uses an export saw a zero-byte delta. +- The flag saves a consumer bytes only when none of an entry's exports are used. `sideEffects` is module-granularity and `splitting: false` emits each entry as a single JS module, so there is no partial drop. Do not advertise it as a tree-shaking win. ## Releases diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js index 9f5e2fa..4da5e06 100644 --- a/scripts/assert-no-import-side-effects.js +++ b/scripts/assert-no-import-side-effects.js @@ -9,17 +9,22 @@ * exercises the built artifacts' import behaviour. * * Detection is deliberately belt-and-braces, because no single mechanism covers - * the ways module evaluation can be observable: named traps for the globals and - * schedulers a refactor would plausibly reach for, and a before/after diff of the - * environment for everything else. What is NOT detected is synchronous - * filesystem work (`fs.writeFileSync` leaves no trace); the no-builtin-imports - * assertion below covers that class at the bundle level instead. + * the ways module evaluation can be observable: named traps for the globals, + * schedulers and process hooks a refactor would plausibly reach for, and a + * before/after diff of the environment by value for everything else. + * + * Known gaps, in rough order of likelihood: synchronous filesystem work leaves + * nothing to observe (the no-builtin-import check below covers that class at the + * bundle level instead); an unhandled promise rejection at module scope needs the + * event loop to turn and so is not seen; mutations to statics rather than + * prototypes (`Object.assign = …`) are not snapshotted; and a symbol-keyed + * addition to globalThis is not either, for the reason given at the snapshot. * * Usage: node scripts/assert-no-import-side-effects.js */ import { spawnSync } from 'child_process' -import { createRequire } from 'module' +import { builtinModules, createRequire } from 'module' import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs' import { fileURLToPath, pathToFileURL } from 'url' import { dirname, join, resolve } from 'path' @@ -54,7 +59,7 @@ const TRAPPED_GLOBALS = [ const TRAPPED_SCHEDULERS = ['setTimeout', 'setInterval', 'setImmediate', 'queueMicrotask'] const TRAPPED_PROCESS_METHODS = ['nextTick', 'on', 'once', 'addListener', 'prependListener'] -/** Patching any of these is a side effect the environment diff reports by name. */ +/** Prototypes whose keys are compared by value, so replacement and deletion show up. */ const WATCHED_PROTOTYPES = ['Object', 'Array', 'String', 'Number', 'Function', 'Promise', 'Date'] /** @@ -66,13 +71,9 @@ const EXTERNALS = ['react', 'react/jsx-runtime'] /** Extensions this script knows how to load; anything else must not be skipped silently. */ const LOADABLE = ['.js', '.cjs', '.mjs'] -const NOT_RUNTIME = /\.(d\.ts|d\.cts|d\.mts|map)$/ - -const BUILTIN_IMPORT = new RegExp( - String.raw`(?:from|require\()\s*['"](?:node:)?` + - '(fs|http|https|net|path|os|child_process|crypto|stream|zlib|dns|tls|worker_threads|vm|module)' + - String.raw`['"]`, -) +const NOT_RUNTIME = /\.(d\.ts|d\.cts|d\.mts|map|json)$/ +const BUILTINS = new Set(builtinModules) +const SPECIFIER = /(?:\bfrom|\brequire|\bimport)\s*\(?\s*['"]([^'"]+)['"]/g function countByType(values) { const counts = {} @@ -80,6 +81,17 @@ function countByType(values) { return counts } +/** Node builtins a bundle imports, by root name. `node:fs/promises` reports as `fs`. */ +function builtinImports(source) { + const found = new Set() + for (const [, specifier] of source.matchAll(SPECIFIER)) { + const bare = specifier.startsWith('node:') ? specifier.slice(5) : specifier + const rootName = bare.split('/')[0] + if (BUILTINS.has(rootName)) found.add(rootName) + } + return [...found] +} + function collectEntries(exportsMap) { const paths = new Set() const walk = (node) => { @@ -96,35 +108,73 @@ function collectEntries(exportsMap) { const entries = [] const unknown = [] for (const path of [...paths].sort()) { - if (NOT_RUNTIME.test(path)) continue + // A wildcard subpath is a pattern, not a module, and package.json is data. + if (path.includes('*') || NOT_RUNTIME.test(path)) continue if (LOADABLE.some((extension) => path.endsWith(extension))) entries.push(path) else unknown.push(path) } return { entries, unknown } } +/** + * Identity token for a property, read from its descriptor so that an accessor's + * getter is never invoked — the traps installed below are accessors, and reading + * them would record a side effect against the module under test. + */ +function describeKeys(target, keys) { + const tokens = new Map() + for (const key of keys) { + const descriptor = Object.getOwnPropertyDescriptor(target, key) + if (!descriptor) continue + const name = typeof key === 'symbol' ? `@@${String(key)}` : key + tokens.set(name, 'get' in descriptor && descriptor.get ? descriptor.get : descriptor.value) + } + return tokens +} + /** Snapshot taken after the traps are installed, so the traps are not reported as changes. */ function snapshotEnvironment() { + const prototypes = new Map() + for (const name of WATCHED_PROTOTYPES) { + const prototype = globalThis[name]?.prototype + if (!prototype) continue + for (const [key, token] of describeKeys(prototype, Reflect.ownKeys(prototype))) { + prototypes.set(`${name}.${key}`, token) + } + prototypes.set(`${name} frozen`, Object.isFrozen(prototype)) + } + return { - globals: Object.getOwnPropertyNames(globalThis), - env: Object.entries(process.env).map(([key, value]) => `${key}=${value}`), - prototypes: WATCHED_PROTOTYPES.flatMap((name) => - Object.getOwnPropertyNames(globalThis[name].prototype).map((key) => `${name}.${key}`), - ), + // String keys only: Node registers symbol-keyed internals on globalThis + // lazily (undici's dispatcher, with an incrementing counter), so reading the + // descriptors here would itself look like a change on the next snapshot. + globals: describeKeys(globalThis, Object.getOwnPropertyNames(globalThis)), + prototypes, + env: new Map(Object.entries(process.env)), + processState: new Map([ + ['cwd', process.cwd()], + ['title', process.title], + ]), resources: countByType(process.getActiveResourcesInfo()), } } function diffEnvironment(before, after) { const changes = [] - const added = (label, from, to) => { - const known = new Set(from) - for (const value of to) if (!known.has(value)) changes.push(`${label} ${value}`) + + const compare = (label, from, to) => { + for (const [key, token] of to) { + // Object.is, not ===, so the NaN global does not read as replaced every run. + if (!from.has(key)) changes.push(`${label} added ${key}`) + else if (!Object.is(from.get(key), token)) changes.push(`${label} replaced ${key}`) + } + for (const key of from.keys()) if (!to.has(key)) changes.push(`${label} removed ${key}`) } - added('added global', before.globals, after.globals) - added('changed process.env', before.env, after.env) - added('patched', before.prototypes, after.prototypes) + compare('global', before.globals, after.globals) + compare('prototype', before.prototypes, after.prototypes) + compare('process.env', before.env, after.env) + compare('process state', before.processState, after.processState) for (const [type, count] of Object.entries(after.resources)) { const extra = count - (before.resources[type] ?? 0) @@ -164,10 +214,13 @@ async function runProbe(target, format, outFile) { } } + // Shaped like a Timeout so a module that stores and unrefs the handle still + // reaches the end of its evaluation, rather than also reporting an import error. + const handle = { unref: () => handle, ref: () => handle, hasRef: () => false, close: () => {} } for (const name of TRAPPED_SCHEDULERS) { globalThis[name] = () => { accesses.push(`${name}()`) - return undefined + return handle } } @@ -224,10 +277,11 @@ function probe(target, format) { const dir = mkdtempSync(join(tmpdir(), 'utm-probe-')) const outFile = join(dir, 'result.json') - // NODE_DEBUG=module makes Node log through the trapped stderr writer, which - // would be reported against the package rather than against the debugger. + // NODE_DEBUG makes Node log through the trapped stderr writer, which would be + // reported against the package rather than against the debugger. const env = { ...process.env } delete env.NODE_DEBUG + delete env.NODE_DEBUG_NATIVE try { const result = spawnSync(process.execPath, [selfPath, '--probe', target, format, outFile], { @@ -241,7 +295,8 @@ function probe(target, format) { try { parsed = JSON.parse(readFileSync(outFile, 'utf-8')) } catch { - const detail = result.stderr?.trim() || result.stdout?.trim() || 'no output' + const detail = + result.error?.message || result.stderr?.trim() || result.stdout?.trim() || 'no output' return { accesses: [], unprotected: [], @@ -265,14 +320,15 @@ function probe(target, format) { } /** - * The check is worthless if the probe cannot report a side effect, so prove on - * every run that each detector fires. Returns the reason it is untrustworthy, or - * null. Add a case here whenever a detector is added. + * The check is worthless if the probe cannot report a side effect, and harmful if + * it reports one that is not there, so prove both on every run. Returns the reason + * it is untrustworthy, or null. Add a case here whenever a detector is added. */ function selfTestFailure() { const dir = mkdtempSync(join(tmpdir(), 'utm-side-effects-')) try { const cases = [ + ['module.exports = 1\n', []], ["console.log('x')\n", ['console.log()']], ["process.stdout.write('x')\n", ['process.stdout.write()']], ['const read = sessionStorage\n', ['read sessionStorage']], @@ -280,27 +336,40 @@ function selfTestFailure() { ['setInterval(() => {}, 1000)\n', ['setInterval()']], ['queueMicrotask(() => {})\n', ['queueMicrotask()']], ["process.on('exit', () => {})\n", ['process.on()']], - ['globalThis.__utmProbeMarker = 1\n', ['added global __utmProbeMarker']], - ["process.env.UTM_PROBE_MARKER = '1'\n", ['changed process.env UTM_PROBE_MARKER=1']], - ['Array.prototype.__utmProbeMarker = 1\n', ['patched Array.__utmProbeMarker']], + ['globalThis.__utmProbeMarker = 1\n', ['global added __utmProbeMarker']], + ['globalThis.structuredClone = () => null\n', ['global replaced structuredClone']], + ["process.env.UTM_PROBE_MARKER = '1'\n", ['process.env added UTM_PROBE_MARKER']], + ['Array.prototype.__utmProbeMarker = 1\n', ['prototype added Array.__utmProbeMarker']], + ['Array.prototype.includes = function () {}\n', ['prototype replaced Array.includes']], + ['delete Array.prototype.at\n', ['prototype removed Array.at']], + ['Object.freeze(Date.prototype)\n', ['prototype replaced Date frozen']], + ["process.chdir('/')\n", ['process state replaced cwd']], ["require('fs').writeSync(1, 'x')\n", ['straight to stdout']], + ["require('node:timers').setInterval(() => {}, 1000)\n", ['active Timeout handle']], ] let index = 0 for (const [body, expectations] of cases) { index += 1 - // Each case runs as CJS so `require` is available to the stdout-bypass case. - const file = join(dir, `impure-${index}.cjs`) + // Each case runs as CJS so `require` is available to the bypass cases. + const file = join(dir, `case-${index}.cjs`) writeFileSync(file, body) const { accesses, unprotected, failure } = probe(file, 'cjs') - if (failure) return `probing known-impure case ${index} errored: ${failure}` + if (failure) return `probing self-test case ${index} errored: ${failure}` if (unprotected.length > 0) { return `could not trap ${unprotected.join(', ')}, so the check is incomplete` } + + if (expectations.length === 0) { + if (accesses.length > 0) { + return `the probe reports side effects for a module that has none (${accesses.join(', ')}), so this environment is too noisy to judge the package` + } + continue + } for (const expected of expectations) { if (!accesses.some((access) => access.includes(expected))) { - return `probe missed "${expected}" in known-impure case ${index} (saw: ${accesses.join(', ') || 'nothing'})` + return `probe missed "${expected}" in self-test case ${index} (saw: ${accesses.join(', ') || 'nothing'})` } } } @@ -318,9 +387,7 @@ function main() { console.error( `Cannot probe these entry points, so they would go unchecked: ${unknown.join(', ')}`, ) - console.error( - `Teach LOADABLE in ${'scripts/assert-no-import-side-effects.js'} how to load them.`, - ) + console.error('Teach LOADABLE in this script how to load them.') process.exit(1) } if (entries.length === 0) { @@ -354,17 +421,16 @@ function main() { problems.push(`${entry}: could not be imported (${failure}).${hint}`) } - const source = (() => { - try { - return readFileSync(target, 'utf-8') - } catch { - return '' - } - })() - const builtin = source.match(BUILTIN_IMPORT) - if (builtin) { + let source = '' + try { + source = readFileSync(target, 'utf-8') + } catch { + // Already reported as an import failure above. + } + const builtins = builtinImports(source) + if (builtins.length > 0) { problems.push( - `${entry}: imports the node builtin "${builtin[1]}", whose calls this check cannot see.`, + `${entry}: imports the node builtin(s) ${builtins.join(', ')}, whose calls this check cannot see.`, ) } } From f7402a83aa075987648d2b3782a2b14ce574fb91 Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 13:05:53 +0100 Subject: [PATCH 7/8] docs: correct the gap list, the externals invariant and a false test claim Three claims were wrong or misleading. "mutations to statics rather than prototypes are not snapshotted" implied prototypes were covered. Only the seven in WATCHED_PROTOTYPES are: Map.prototype.x and Error.prototype.toJSON both pass clean, as does any process state beyond cwd and title. The gap list now says so, and loses the unverifiable "in rough order of likelihood". The externals comment pointed at tsup.config.ts as the source of truth. That config externalises only react, while this list needs react/jsx-runtime too, because tsup matches a prefix and this script requires each specifier literally - and tsup additionally externalises every dependency and peerDependency without the config naming them. A maintainer following the old comment would have missed a new runtime dependency. It now states the invariant the list actually has to satisfy. CLAUDE.md claimed a cached module-level read would make the suite order-dependent. It would not: vitest evaluates a test file's imports at collection, before any beforeEach, so such a read captures jsdom's original every time and fails deterministically in any order. The throw-on-a-server reason already carried the rule, so the second rationale is gone, along with a brittle count of guard sites - one of the 13 was in a markdown file - and a copy of reasoning the script header already holds. Also: the probe exits for the parent's 30s timeout rather than stalling it indefinitely; the undici symbols carry a version suffix, not a counter; the self-test directive now excludes the builtin check, which runs in the parent; and NODE_DEBUG_NATIVE is no longer deleted, since its output is C++-side and never reaches the trapped writer. --- CLAUDE.md | 2 +- scripts/assert-no-import-side-effects.js | 28 +++++++++++++----------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1a57813..d2d1dd4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,7 +17,7 @@ - All URL-facing operations convert keys to `snake_case` internally, regardless of the consumer's `keyFormat` setting. - `check:package` runs attw with `--profile node16`, so the `node10` row is still printed but every cell is marked `(ignored)` and it cannot affect the exit code. It is red for `/react` and `/server` because legacy resolution cannot see `exports` subpaths; that is a declared non-target, not a bug to fix. Do not add `typesVersions`: it would green the row for types only. - publint and attw each catch what the other misses. attw exits 0 on a tarball containing no declarations at all, so publint is what catches missing or unpublished files; publint passes a correctly-wired `exports` map whose declarations import chunks the build never emitted, so attw is what catches that. Dropping either silently loses a class of failure. -- `scripts/assert-no-import-side-effects.js` guards `"sideEffects": false`, which publint reports on but neither publint nor attw verifies. Module-scope reads of the browser globals fail it, not only writes: `window.location` there throws on a server, and the trap cannot tell that from a harmless `typeof window`. Detect the environment at call time, as all 13 guard sites in `src/` do; `__tests__/setup.ts` re-stubs those globals per test, so a cached module-level read also makes the suite order-dependent. It also fails any entry that imports a node builtin, since it cannot see what such code does at import; no entry needs one today. +- `scripts/assert-no-import-side-effects.js` guards `"sideEffects": false` in `check:package`, and nothing else verifies that claim. It fails a module-scope *read* of a browser global, not only a write, and fails any entry that imports a node builtin, since it cannot see what such code does at import; no entry needs one today. Detect the environment at call time, as every site in `src/` does; the script header gives the reasoning. - The flag saves a consumer bytes only when none of an entry's exports are used. `sideEffects` is module-granularity and `splitting: false` emits each entry as a single JS module, so there is no partial drop. Do not advertise it as a tree-shaking win. ## Releases diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js index 4da5e06..c823b62 100644 --- a/scripts/assert-no-import-side-effects.js +++ b/scripts/assert-no-import-side-effects.js @@ -13,12 +13,12 @@ * schedulers and process hooks a refactor would plausibly reach for, and a * before/after diff of the environment by value for everything else. * - * Known gaps, in rough order of likelihood: synchronous filesystem work leaves - * nothing to observe (the no-builtin-import check below covers that class at the - * bundle level instead); an unhandled promise rejection at module scope needs the - * event loop to turn and so is not seen; mutations to statics rather than - * prototypes (`Object.assign = …`) are not snapshotted; and a symbol-keyed - * addition to globalThis is not either, for the reason given at the snapshot. + * Known gaps: synchronous filesystem work leaves nothing to observe (the + * no-builtin-import check below covers that class at the bundle level instead), + * and an unhandled promise rejection at module scope needs the event loop to turn + * and so is not seen. The snapshot does not reach statics (`Object.assign = …`), + * prototypes outside WATCHED_PROTOTYPES (`Map.prototype.x = …`), process state + * beyond cwd and title, or symbol-keyed additions to globalThis. * * Usage: node scripts/assert-no-import-side-effects.js */ @@ -64,8 +64,10 @@ const WATCHED_PROTOTYPES = ['Object', 'Array', 'String', 'Number', 'Function', ' /** * Imported before the traps go up, so their own evaluation is not attributed - * here. Must stay in step with `external` in tsup.config.ts: an external missing - * from this list has its side effects blamed on our entry. + * here. Must list every bare specifier the built entries import, subpaths + * included: tsup externalises `external` from its config plus every dependency + * and peerDependency, and an external missing from this list has its side + * effects blamed on our entry. */ const EXTERNALS = ['react', 'react/jsx-runtime'] @@ -146,8 +148,8 @@ function snapshotEnvironment() { return { // String keys only: Node registers symbol-keyed internals on globalThis - // lazily (undici's dispatcher, with an incrementing counter), so reading the - // descriptors here would itself look like a change on the next snapshot. + // lazily (undici's dispatcher), so reading the descriptors here would itself + // look like a change on the next snapshot. globals: describeKeys(globalThis, Object.getOwnPropertyNames(globalThis)), prototypes, env: new Map(Object.entries(process.env)), @@ -263,7 +265,7 @@ async function runProbe(target, format, outFile) { writeFileSync(outFile, JSON.stringify({ accesses, unprotected, failure, failureCode })) // A timer, socket or listener started during evaluation would otherwise keep - // this process alive and stall the parent instead of failing it. + // this process alive until the parent's 30s timeout. process.exit(0) } @@ -281,7 +283,6 @@ function probe(target, format) { // reported against the package rather than against the debugger. const env = { ...process.env } delete env.NODE_DEBUG - delete env.NODE_DEBUG_NATIVE try { const result = spawnSync(process.execPath, [selfPath, '--probe', target, format, outFile], { @@ -322,7 +323,8 @@ function probe(target, format) { /** * The check is worthless if the probe cannot report a side effect, and harmful if * it reports one that is not there, so prove both on every run. Returns the reason - * it is untrustworthy, or null. Add a case here whenever a detector is added. + * it is untrustworthy, or null. Add a case here whenever a detector inside the + * probe is added; the builtin-import check runs in the parent and has none. */ function selfTestFailure() { const dir = mkdtempSync(join(tmpdir(), 'utm-side-effects-')) From 1beeba105f1bbc41bd5457e47787eecb8378158d Mon Sep 17 00:00:00 2001 From: Jack Misner Date: Fri, 23 Oct 2026 13:09:14 +0100 Subject: [PATCH 8/8] fix: refuse to run on Node minors that read window themselves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Node 20.0-20.17 and 22.0-22.9 a clean build reported `dist/react/index.js: evaluated on import — read window`. Reproduced on 20.14.0. Nothing in the package does it: the react entry is ESM importing a CJS dependency, and the first such import in a process initialises Node's cjs-module-lexer, which reads `window` on those minors. The read lands after the traps go up, so it is attributed to the entry that triggered it. Warming the lexer first does not fix it. Importing a throwaway .cjs through the ESM loader before taking the baseline still failed on 20.14.0 and added a CloseReq handle that lands after the baseline, so it traded one false positive for two. The check now states the requirement and exits instead. A precondition a reader can act on is worth more than a side-effect claim that is not true, and CI is unaffected: the workflows pin major versions, which float to the latest minor. Development below Node 22 is already out of reach, since jsdom needs 22+. Also qualifies the undici note as Node 22+, where it was measured, and adds the setter-only accessor to the gap list. --- scripts/assert-no-import-side-effects.js | 29 +++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/scripts/assert-no-import-side-effects.js b/scripts/assert-no-import-side-effects.js index c823b62..c0a710e 100644 --- a/scripts/assert-no-import-side-effects.js +++ b/scripts/assert-no-import-side-effects.js @@ -18,7 +18,8 @@ * and an unhandled promise rejection at module scope needs the event loop to turn * and so is not seen. The snapshot does not reach statics (`Object.assign = …`), * prototypes outside WATCHED_PROTOTYPES (`Map.prototype.x = …`), process state - * beyond cwd and title, or symbol-keyed additions to globalThis. + * beyond cwd and title, symbol-keyed additions to globalThis, or an accessor + * whose setter alone is swapped. * * Usage: node scripts/assert-no-import-side-effects.js */ @@ -77,6 +78,20 @@ const NOT_RUNTIME = /\.(d\.ts|d\.cts|d\.mts|map|json)$/ const BUILTINS = new Set(builtinModules) const SPECIFIER = /(?:\bfrom|\brequire|\bimport)\s*\(?\s*['"]([^'"]+)['"]/g +/** + * Node 20.0-20.17 and 22.0-22.9 read `window` while initialising the CJS module + * lexer, which the first ESM import of a CJS dependency triggers — reported as a + * side effect of the entry that triggered it. Refusing to run beats accusing the + * package of something Node did. Warming the lexer first does not help: it leaves + * a CloseReq handle that lands after the baseline instead. + */ +function nodeTooOldToProbe() { + const [major, minor] = process.versions.node.split('.').map(Number) + if (major === 20 && minor < 18) return '20.18.0' + if (major === 22 && minor < 10) return '22.10.0' + return null +} + function countByType(values) { const counts = {} for (const value of values) counts[value] = (counts[value] ?? 0) + 1 @@ -148,8 +163,8 @@ function snapshotEnvironment() { return { // String keys only: Node registers symbol-keyed internals on globalThis - // lazily (undici's dispatcher), so reading the descriptors here would itself - // look like a change on the next snapshot. + // lazily (undici's dispatcher, on Node 22+), so reading the descriptors here + // would itself look like a change on the next snapshot. globals: describeKeys(globalThis, Object.getOwnPropertyNames(globalThis)), prototypes, env: new Map(Object.entries(process.env)), @@ -382,6 +397,14 @@ function selfTestFailure() { } function main() { + const needed = nodeTooOldToProbe() + if (needed) { + console.error( + `This check needs Node >= ${needed}; running ${process.versions.node}, which reads \`window\` while initialising the CJS module lexer.`, + ) + process.exit(1) + } + const pkg = JSON.parse(readFileSync(join(root, 'package.json'), 'utf-8')) const { entries, unknown } = collectEntries(pkg.exports)