Declare side-effecting modules so bundlers have better hints for what can be tree-shaken

.github/workflows/ci-jobs.yml

@@ -74,6 +74,8 @@ jobs:
       - name: build
         run: pnpm build
       - uses: wyvox/action-no-git-diff@6d1f5759a221e2ea447974af795e395672e33328 # v1.0.1
+      - name: check tree-shaking
+        run: pnpm test:tree-shake
 
   basic-test:
     name: Basic Test

bin/check-tree-shake.mjs

@@ -0,0 +1,196 @@
+/* eslint-disable no-console, n/no-process-exit */
+import { check } from 'agadoo';
+import fs from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
+
+// Entries that fully tree-shake today. A regression here means a new
+// top-level side effect was introduced — fix it or move the entry to
+// KNOWN_IMPURE_ENTRIES (and update package.json sideEffects accordingly).
+const KNOWN_PURE_ENTRIES = [
+  '@ember/-internals/browser-environment',
+  '@ember/-internals/error-handling',
+  '@ember/-internals/owner',
+  '@ember/-internals/utility-types',
+  '@ember/deprecated-features',
+  '@ember/destroyable',
+  '@ember/owner',
+  '@ember/reactive',
+  '@ember/template-compilation',
+  '@ember/test',
+  '@ember/version',
+  '@glimmer/destroyable',
+  '@glimmer/encoder',
+  '@glimmer/env',
+  '@glimmer/global-context',
+  '@glimmer/owner',
+  '@glimmer/util',
+  '@glimmer/vm',
+  '@glimmer/wire-format',
+  '@simple-dom/document',
+  'backburner.js',
+  'dag-map',
+  'route-recognizer',
+];
+
+// Entries that have known top-level side effects today (manager
+// registrations, validator state, backburner instance, etc.). These are
+// tracked so that improvements get noticed: if one of these starts
+// passing agadoo, promote it to KNOWN_PURE_ENTRIES.
+const KNOWN_IMPURE_ENTRIES = [
+  '@ember/-internals/container',
+  '@ember/-internals/deprecations',
+  '@ember/-internals/environment',
+  '@ember/-internals/glimmer',
+  '@ember/-internals/meta',
+  '@ember/-internals/metal',
+  '@ember/-internals/routing',
+  '@ember/-internals/runtime',
+  '@ember/-internals/string',
+  '@ember/-internals/utils',
+  '@ember/-internals/views',
+  '@ember/application',
+  '@ember/array',
+  '@ember/canary-features',
+  '@ember/component',
+  '@ember/controller',
+  '@ember/debug',
+  '@ember/engine',
+  '@ember/enumerable',
+  '@ember/helper',
+  '@ember/instrumentation',
+  '@ember/modifier',
+  '@ember/object',
+  '@ember/renderer',
+  '@ember/routing',
+  '@ember/runloop',
+  '@ember/service',
+  '@ember/template',
+  '@ember/template-compiler',
+  '@ember/template-factory',
+  '@ember/utils',
+  '@glimmer/manager',
+  '@glimmer/node',
+  '@glimmer/opcode-compiler',
+  '@glimmer/program',
+  '@glimmer/reference',
+  '@glimmer/runtime',
+  '@glimmer/tracking',
+  '@glimmer/validator',
+  'ember-template-compiler',
+  'ember-testing',
+  'router_js',
+  'rsvp',
+];
+
+function findAllEntries() {
+  const distRoot = path.join(root, 'dist/prod/packages');
+  const entries = [];
+  function walk(dir, prefix) {
+    for (const name of fs.readdirSync(dir)) {
+      const full = path.join(dir, name);
+      const stat = fs.statSync(full);
+      if (stat.isDirectory()) {
+        const indexPath = path.join(full, 'index.js');
+        if (fs.existsSync(indexPath)) {
+          entries.push(prefix ? `${prefix}/${name}` : name);
+        } else {
+          walk(full, prefix ? `${prefix}/${name}` : name);
+        }
+      }
+    }
+  }
+  walk(distRoot, '');
+  return entries.sort();
+}
+
+const allEntries = findAllEntries();
+const tracked = new Set([...KNOWN_PURE_ENTRIES, ...KNOWN_IMPURE_ENTRIES]);
+const untracked = allEntries.filter((e) => !tracked.has(e));
+const removed = [...tracked].filter((e) => !allEntries.includes(e));
+
+const purityRegressed = [];
+const newlyPure = [];
+
+for (const entry of KNOWN_PURE_ENTRIES) {
+  if (!allEntries.includes(entry)) continue;
+  const file = path.join(root, 'dist/prod/packages', entry, 'index.js');
+  const result = await check(file);
+  if (result.shaken) {
+    console.log(`  ✓ ${entry} (pure)`);
+  } else {
+    console.error(`  ✗ ${entry} (expected pure, has side effects)`);
+    purityRegressed.push(entry);
+  }
+}
+
+for (const entry of KNOWN_IMPURE_ENTRIES) {
+  if (!allEntries.includes(entry)) continue;
+  const file = path.join(root, 'dist/prod/packages', entry, 'index.js');
+  const result = await check(file);
+  if (result.shaken) {
+    console.error(`  ! ${entry} (expected impure, now pure — promote it!)`);
+    newlyPure.push(entry);
+  } else {
+    console.log(`  · ${entry} (impure, as expected)`);
+  }
+}
+
+let exitCode = 0;
+
+if (purityRegressed.length > 0) {
+  console.error(
+    `\n${purityRegressed.length} entr${purityRegressed.length === 1 ? 'y' : 'ies'} regressed (was pure, now impure):`
+  );
+  for (const entry of purityRegressed) console.error(`  - ${entry}`);
+  console.error(
+    `\nA new top-level side effect was introduced. Either:\n` +
+      `  1. Remove the side effect (preferred), or\n` +
+      `  2. If the side effect is intentional, move the entry from\n` +
+      `     KNOWN_PURE_ENTRIES to KNOWN_IMPURE_ENTRIES in\n` +
+      `     bin/check-tree-shake.mjs.`
+  );
+  exitCode = 1;
+}
+
+if (newlyPure.length > 0) {
+  console.error(
+    `\n${newlyPure.length} entr${newlyPure.length === 1 ? 'y' : 'ies'} unexpectedly tree-shake (was impure, now pure):`
+  );
+  for (const entry of newlyPure) console.error(`  - ${entry}`);
+  console.error(
+    `\nThis is good news — those packages got smaller. Promote them by\n` +
+      `moving them from KNOWN_IMPURE_ENTRIES to KNOWN_PURE_ENTRIES in\n` +
+      `bin/check-tree-shake.mjs.`
+  );
+  exitCode = 1;
+}
+
+if (untracked.length > 0) {
+  console.error(`\n${untracked.length} entr${untracked.length === 1 ? 'y' : 'ies'} not tracked:`);
+  for (const entry of untracked) console.error(`  - ${entry}`);
+  console.error(
+    `\nNew package(s) appeared. Run agadoo manually and add each to either\n` +
+      `KNOWN_PURE_ENTRIES or KNOWN_IMPURE_ENTRIES in bin/check-tree-shake.mjs.`
+  );
+  exitCode = 1;
+}
+
+if (removed.length > 0) {
+  console.error(
+    `\n${removed.length} tracked entr${removed.length === 1 ? 'y' : 'ies'} no longer build:`
+  );
+  for (const entry of removed) console.error(`  - ${entry}`);
+  console.error(`\nRemove the stale entries from bin/check-tree-shake.mjs.`);
+  exitCode = 1;
+}
+
+if (exitCode === 0) {
+  console.log(
+    `\n${KNOWN_PURE_ENTRIES.length} pure + ${KNOWN_IMPURE_ENTRIES.length} impure entries match expectations.`
+  );
+}
+
+process.exit(exitCode);

package.json

@@ -16,6 +16,7 @@
     },
     "./package.json": "./package.json"
   },
+  "sideEffects": false,
   "homepage": "https://emberjs.com/",
   "bugs": {
     "url": "https://github.com/emberjs/ember.js/issues"
@@ -53,6 +54,7 @@
     "test": "testem ci -f testem.js --host 127.0.0.1 --port 13141",
     "test:blueprints": "mocha node-tests/blueprints/**/*-test.js",
     "test:node": "qunit tests/node/**/*-test.js",
+    "test:tree-shake": "node bin/check-tree-shake.mjs",
     "test:browserstack": "node bin/run-browserstack-tests.js",
     "test:wip": "vite build --mode development --minify false && testem ci",
     "type-check:internals": "tsc --noEmit",
@@ -98,6 +100,7 @@
     "@tsconfig/ember": "3.0.8",
     "@types/qunit": "^2.19.4",
     "@types/rsvp": "^4.0.4",
+    "agadoo": "^3.0.0",
     "ast-types": "^0.14.2",
     "auto-dist-tag": "^2.1.1",
     "babel-plugin-debug-macros": "1.0.0",
@@ -157,7 +160,8 @@
       "esbuild"
     ],
     "patchedDependencies": {
-      "@tracerbench/core@8.0.1": "patches/@tracerbench__core@8.0.1.patch"
+      "@tracerbench/core@8.0.1": "patches/@tracerbench__core@8.0.1.patch",
+      "agadoo": "patches/agadoo.patch"
     }
   },
   "peerDependencies": {

patches/agadoo.patch

@@ -0,0 +1,22 @@
+diff --git a/index.js b/index.js
+index 53354a5de59bb26283bb2cedb6ff5ff0ca6a1fa1..28777e32a9960785fcc31d79ade2a14b200f0cfe 100644
+--- a/index.js
++++ b/index.js
+@@ -25,7 +25,7 @@ export async function check(input) {
+ 	const { code } = result.output[0];
+
+ 	const ast = acorn.parse(code, {
+-		ecmaVersion: 11,
++		ecmaVersion: 'latest',
+ 		sourceType: 'module'
+ 	});
+
+@@ -33,7 +33,7 @@ export async function check(input) {
+ 		return node.type !== 'ImportDeclaration';
+ 	});
+
+-	console.log(code);
++// console.log removed by patch
+
+ 	return {
+ 		shaken: nodes.length === 0

rollup.config.mjs

@@ -100,6 +100,9 @@ function sharedESMConfig({ input, debugMacrosMode, includePackageMeta = false })
   return {
     onLog: handleRollupWarnings,
     input,
+    treeshake: {
+      moduleSideEffects: moduleHasSideEffects,
+    },
     output: {
       format: 'es',
       dir: outputDir,
@@ -111,6 +114,18 @@ function sharedESMConfig({ input, debugMacrosMode, includePackageMeta = false })
   };
 }
 
+// `@glimmer/debug` declares `sideEffects: false` in its own package.json,
+// but rolldown loses that signal when files from it get co-bundled into
+// shared chunks alongside non-pure code. Re-asserting purity at the
+// chunk-level callback recovers ~8 KB raw on the hello-world bundle by
+// letting unused imports of `@glimmer/debug` content drop out of those
+// chunks. Add other packages here only after measuring an actual
+// reduction.
+function moduleHasSideEffects(id) {
+  if (id.includes('/packages/@glimmer/debug/')) return false;
+  return true;
+}
+
 function glimmerSyntaxESM() {
   return {
     onLog: handleRollupWarnings,