Tighten sideEffects list; remove refactor-context comments

NullVoxPopuli · claude · NullVoxPopuli · commit 1f71dedc010f · 2026-05-02T21:37:14.000-04:00
Two pieces of polish from the user's review:

1. The `sideEffects` field in package.json was over-broad — listing
   whole directories (`**/manager/**`, `**/validator/**`, etc.) when
   only a handful of files in those trees actually have top-level side
   effects. Replaced the directory globs with the explicit list of
   files that contain top-level calls (registrations, opcode
   `APPEND_OPCODES.add(...)`, `setGlobalContext`, `_backburner.on`,
   etc.). Hello-world stays at 134.12 KB / 42.92 KB.

2. Removed comments in the refactored files that explained the
   refactor itself ("extracted from X for tree-shaking", "kept
   separate so Y", "back-compat re-export"). That kind of context
   belongs in the PR description and commit messages, not the source
   tree.

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/package.json b/package.json
@@ -18,28 +18,29 @@
   },
   "sideEffects": [
     "./dist/dev/**",
-    "**/setup-registry*",
-    "**/register-routing-keywords*",
-    "**/register-curly-component*",
-    "**/debug-render-tree-register*",
-    "**/-internals/glimmer/lib/environment*",
-    "**/runtime/lib/environment*",
-    "**/-internals/runtime/lib/ext/rsvp*",
-    "**/runtime/lib/component/template-only*",
-    "**/runtime/lib/vm/low-level*",
-    "**/runtime/lib/compiled/opcodes/**",
-    "**/runtime/lib/helpers/**",
-    "**/runtime/lib/modifiers/**",
-    "**/-internals/glimmer/lib/components/**",
-    "**/-internals/views/lib/system/event_dispatcher*",
-    "**/runloop/index*",
-    "**/runloop/-private/**",
-    "**/manager/**",
-    "**/validator/**",
-    "**/global-context/**",
-    "**/destroyable/**",
-    "**/canary-features/**",
-    "**/-internals/environment/**"
+    "**/@ember/-internals/glimmer/lib/setup-registry.js",
+    "**/@ember/-internals/glimmer/lib/register-curly-component.js",
+    "**/@ember/-internals/glimmer/lib/syntax/register-routing-keywords.js",
+    "**/@ember/-internals/glimmer/lib/environment.js",
+    "**/@ember/-internals/glimmer/lib/renderer.js",
+    "**/@ember/-internals/glimmer/lib/helper.js",
+    "**/@ember/-internals/glimmer/lib/helpers/element.js",
+    "**/@ember/-internals/glimmer/lib/components/input.js",
+    "**/@ember/-internals/glimmer/lib/components/textarea.js",
+    "**/@ember/-internals/glimmer/lib/components/link-to.js",
+    "**/@ember/-internals/runtime/lib/ext/rsvp.js",
+    "**/@glimmer/runtime/lib/debug-render-tree-register.js",
+    "**/@glimmer/runtime/lib/component/template-only.js",
+    "**/@glimmer/runtime/lib/compiled/opcodes/expressions.js",
+    "**/@glimmer/runtime/lib/compiled/opcodes/vm.js",
+    "**/@glimmer/runtime/lib/compiled/opcodes/debugger.js",
+    "**/@glimmer/runtime/lib/compiled/opcodes/content.js",
+    "**/@glimmer/runtime/lib/compiled/opcodes/component.js",
+    "**/@glimmer/runtime/lib/compiled/opcodes/dom.js",
+    "**/@glimmer/runtime/lib/compiled/opcodes/lists.js",
+    "**/@glimmer/runtime/lib/vm/low-level.js",
+    "**/@glimmer/validator/index.js",
+    "**/@glimmer/validator/lib/validators.js"
   ],
   "homepage": "https://emberjs.com/",
   "bugs": {
diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly-symbols.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly-symbols.ts
@@ -1,11 +1,3 @@
-// Symbols and the cheap brand-based `isCurlyManager` predicate, kept in
-// their own module so consumers (e.g. the resolver) can identify the
-// curly component manager without statically pulling in the full
-// `./curly` module — that file carries the entire classic component
-// lifecycle plus its transitive deps (~17 KB raw plus the @ember/object
-// graph). With this split, `renderComponent`-only bundles never load
-// `./curly` at all.
-
 export const DIRTY_TAG: unique symbol = Symbol('DIRTY_TAG');
 export const IS_DISPATCHING_ATTRS: unique symbol = Symbol('IS_DISPATCHING_ATTRS');
 export const BOUNDS: unique symbol = Symbol('BOUNDS');
diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts
@@ -70,10 +70,6 @@ export function getComponentCapturedArgs(
   return COMPONENT_ARGS_MAP.get(component);
 }
 
-// Re-exported from `./curly-symbols` to keep import sites unchanged.
-// The actual symbols live there so consumers that only need the symbols
-// (e.g. the renderer's resolver, classic-renderer's `getBounds`) don't
-// have to load the full curly component manager.
 export { BOUNDS, DIRTY_TAG, IS_DISPATCHING_ATTRS } from './curly-symbols';
 
 const EMBER_VIEW_REF = createPrimitiveRef('ember-view');
@@ -565,7 +561,4 @@ const CURLY_CAPABILITIES: InternalComponentCapabilities = {
 export const CURLY_COMPONENT_MANAGER = new CurlyComponentManager();
 (CURLY_COMPONENT_MANAGER as unknown as Record<symbol, boolean>)[CURLY_COMPONENT_BRAND] = true;
 
-// Re-exported for back-compat — `isCurlyManager` and the symbols now live
-// in `./curly-symbols` so that consumers who only need the brand check
-// don't pull in the rest of this module.
 export { CURLY_COMPONENT_BRAND, isCurlyManager } from './curly-symbols';
diff --git a/packages/@ember/-internals/glimmer/lib/component.ts b/packages/@ember/-internals/glimmer/lib/component.ts
@@ -1680,13 +1680,4 @@ class Component<S = unknown>
   }
 }
 
-// `Component.reopenClass({ positionalParams: [] })` and the curly component
-// manager registration that used to live here now happen in
-// `./register-curly-component`, imported as a side effect by classic-app
-// setup (`./setup-registry`). Splitting the side effect out lets bundlers
-// tree-shake the classic `Component` class out of bundles that don't
-// otherwise need it — e.g. the hello-world app that only uses
-// `setComponentManager` / `capabilities` from `@ember/component` via
-// `@glimmer/component`.
-
 export default Component;
diff --git a/packages/@ember/-internals/glimmer/lib/register-curly-component.ts b/packages/@ember/-internals/glimmer/lib/register-curly-component.ts
@@ -1,15 +1,3 @@
-// Side-effect-only file. Registers the classic curly component manager
-// for `Component` and seeds `positionalParams` on the base class.
-//
-// Kept separate from `./component` so that file's top-level evaluation
-// has no side effects — bundlers can then tree-shake the classic
-// `Component` class out of any bundle that doesn't otherwise need it
-// (e.g. the hello-world that only pulls `setComponentManager` /
-// `capabilities` from `@ember/component` via `@glimmer/component`).
-//
-// Classic apps opt back in to the registration via `./setup-registry`,
-// which imports this module for its side effect.
-
 import { setInternalComponentManager } from '@glimmer/manager/lib/internal/api';
 
 import Component from './component';
diff --git a/packages/@ember/-internals/glimmer/lib/renderer.ts b/packages/@ember/-internals/glimmer/lib/renderer.ts
@@ -34,12 +34,6 @@ import ResolverImpl from './resolver';
 
 export type IBuilder = (env: Environment, cursor: Cursor) => TreeBuilder;
 
-/**
- * Common shape of root states tracked by `RendererState`. Both
- * `ComponentRootState` (defined here) and `ClassicRootState` (defined in
- * `./classic-renderer`) implement this so the renderer state can manage
- * either kind without statically depending on classic-only code.
- */
 export interface RootState {
   readonly type: 'classic' | 'component';
   readonly destroyed: boolean;
@@ -670,7 +664,4 @@ export class BaseRenderer {
   }
 }
 
-// Re-export classic-only types so existing import sites that did
-// `import { Renderer, DynamicScope, View } from '.../renderer'` keep working.
-// New code should import these directly from `./classic-renderer`.
 export type { DynamicScope, Renderer, View } from './classic-renderer';
diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts
@@ -96,12 +96,6 @@ const BUILTIN_KEYWORD_HELPERS: Record<string, object> = {
   '-in-el-null': inElementNullCheckHelper,
 };
 
-/**
- * Register an additional built-in keyword helper (e.g. `{{outlet}}`,
- * `{{mount}}`). Kept separate so the renderer doesn't statically depend on
- * routing/engine infrastructure — callers that need those helpers must opt
- * in by importing the registration module (`./syntax/register-routing-keywords`).
- */
 export function registerBuiltInKeywordHelper(name: string, helper: object): void {
   BUILTIN_KEYWORD_HELPERS[name] = helper;
   BUILTIN_HELPERS[name] = helper;
diff --git a/packages/@ember/-internals/glimmer/lib/setup-registry.ts b/packages/@ember/-internals/glimmer/lib/setup-registry.ts
@@ -10,19 +10,6 @@ import { Renderer } from './classic-renderer';
 import OutletTemplate from './templates/outlet';
 import RootTemplate from './templates/root';
 import OutletView from './views/outlet';
-// Side-effect imports. Each keeps classic-only infrastructure off the
-// `renderComponent`-only path:
-//
-// - `register-routing-keywords` registers `-mount` and `-outlet` as
-//   built-in keyword helpers (pulls in routing/engine).
-// - `@glimmer/runtime/.../debug-render-tree-register` registers the
-//   `DebugRenderTreeImpl` factory so `delegate.enableDebugTooling: true`
-//   actually produces a render tree (used by Ember Inspector). Without
-//   this import, `env.debugRenderTree` stays `undefined`.
-// - `register-curly-component` registers the classic component manager
-//   for the `Component` base class and seeds `positionalParams`. Without
-//   this import the classic `Component` class is dead code as far as
-//   `setComponentManager`/`capabilities` consumers are concerned.
 import './syntax/register-routing-keywords';
 import '@glimmer/runtime/lib/debug-render-tree-register';
 import './register-curly-component';
diff --git a/packages/@ember/-internals/glimmer/lib/utils/to-bool.ts b/packages/@ember/-internals/glimmer/lib/utils/to-bool.ts
@@ -5,11 +5,6 @@ import { isEmberArray } from '@ember/array/-internals';
 import { isProxy } from '@ember/-internals/utils/lib/is_proxy';
 import { consumeTag } from '@glimmer/validator/lib/tracking';
 
-// Lightweight array predicate — covers native arrays plus anything branded
-// via `setEmberArray` (which is what `EmberArray.init` does, so any
-// instance of an EmberArray-mixed class is branded). Avoids the full
-// `@ember/array/index` graph (Mixin, Enumerable, Observable, computed)
-// just to test array-ness for `{{#if}}`.
 function isArrayLike(obj: unknown): obj is ArrayLike<unknown> {
   return Array.isArray(obj) || isEmberArray(obj);
 }
diff --git a/packages/@ember/-internals/meta/lib/meta.ts b/packages/@ember/-internals/meta/lib/meta.ts
@@ -267,15 +267,6 @@ export class Meta {
     return undefined;
   }
 
-  // The mixin-tracking helpers (`addMixin` / `hasMixin` / `forEachMixins`)
-  // moved to `@ember/object/mixin` as standalone functions
-  // (`metaAddMixin` / `metaHasMixin` / `metaForEachMixins`). They poke
-  // directly at the public `_mixins` / `parent` fields here. Splitting
-  // them out keeps the `Meta` class — which is reachable from the
-  // renderer via the property accessor + tag chain — free of references
-  // to the classic `Mixin` machinery, so bundles that don't actually use
-  // mixins can tree-shake `@ember/object/mixin` away.
-
   /** @internal */
   writeDescriptors(subkey: string, value: any) {
     assert(
diff --git a/packages/@ember/-internals/runtime/lib/mixins/-proxy.ts b/packages/@ember/-internals/runtime/lib/mixins/-proxy.ts
@@ -19,9 +19,6 @@ import type { Tag } from '@glimmer/interfaces';
 import { combine } from '@glimmer/validator/lib/validators';
 import { tagFor, tagMetaFor } from '@glimmer/validator/lib/meta';
 
-// `contentFor` was extracted to `./content-for` so the `each-in` helper
-// (reachable from the renderer) can use it without dragging in this
-// file's `Mixin.create(...)` graph. Re-exported here for back-compat.
 import { contentFor } from './content-for';
 export { contentFor };
 
diff --git a/packages/@ember/-internals/runtime/lib/mixins/content-for.ts b/packages/@ember/-internals/runtime/lib/mixins/content-for.ts
@@ -4,15 +4,10 @@ import type { UpdatableTag } from '@glimmer/interfaces';
 import { UPDATE_TAG as updateTag } from '@glimmer/validator/lib/validators';
 import type ProxyMixin from './-proxy';
 
-// Extracted from `./-proxy` so that consumers (notably the `each-in` helper
-// reachable from the renderer) don't have to drag in `@ember/object/mixin`
-// and the `ProxyMixin = Mixin.create(...)` side effect — that's the entry
-// point to the entire EmberObject/PrototypeMixin pyramid.
-
 export function contentFor<T>(proxy: ProxyMixin<T>): T | null {
   let content = get(proxy, 'content');
-  // SAFETY: matches the original cast in -proxy.ts; @glimmer/validator
-  // doesn't expose enough public types for an assertion here.
+  // SAFETY: @glimmer/validator doesn't expose enough public types for an
+  // assertion here.
   updateTag(tagForObject(proxy) as UpdatableTag, tagForObject(content));
   return content;
 }
diff --git a/packages/@ember/instrumentation/index.ts b/packages/@ember/instrumentation/index.ts
@@ -7,12 +7,6 @@ import {
   type Subscriber,
 } from './lib/internal-instrument';
 
-// Re-export with `export ... from` (rather than re-binding via local
-// import + export) so the no-barrel-imports lint rule traces consumers
-// through to `./lib/internal-instrument` and rewrites their imports to
-// the deep path. That keeps the heavy `instrument` / `subscribe` /
-// `unsubscribe` machinery defined below out of bundles that only need
-// the hot path.
 export {
   _instrumentStart,
   flaggedInstrument,
diff --git a/packages/@ember/instrumentation/lib/internal-instrument.ts b/packages/@ember/instrumentation/lib/internal-instrument.ts
@@ -1,12 +1,3 @@
-// Hot-path bits of `@ember/instrumentation`, factored out so internal
-// callers (the resolver, the curly component manager, view states) can
-// import `_instrumentStart` without dragging in the full subscribe /
-// unsubscribe / instrument machinery — most of which is dead code unless
-// something actually subscribes (e.g. Ember Inspector).
-//
-// `subscribers` and `cache` live here; `index.ts` imports and mutates
-// them via `subscribe` / `unsubscribe` / `reset`.
-
 import { ENV } from '@ember/-internals/environment/lib/env';
 import { assert } from '@ember/debug';
 
@@ -56,9 +47,7 @@ const time = (): number => performance.now();
 export const NOOP = (): void => {};
 
 // `flaggedInstrument` historically wrapped a callback in conditionally-
-// enabled instrumentation; today it's a thin pass-through. Lives here so
-// hot-path callers (the views state machine) can deep-import it without
-// pulling in the rest of `index.ts`.
+// enabled instrumentation; today it's a thin pass-through.
 export function flaggedInstrument<Result>(
   _name: string,
   _payload: object,
diff --git a/packages/@ember/object/mixin.ts b/packages/@ember/object/mixin.ts
@@ -7,12 +7,6 @@ import { meta as metaFor, peekMeta } from '@ember/-internals/meta/lib/meta';
 import { isDestroyed } from '@glimmer/destroyable';
 import toString from '@ember/-internals/utils/lib/to-string';
 
-// Mixin-tracking helpers — moved out of `Meta` so the renderer-only
-// path (which reaches `Meta` through the tag/property graph) doesn't
-// pull a reference to `Mixin` into the bundle. They poke at `Meta`'s
-// public `_mixins` / `parent` fields directly.
-
- 
 function metaAddMixin(meta: Meta, mixin: any): void {
   assert(
     isDestroyed(meta.source as object)
diff --git a/packages/@glimmer/runtime/lib/debug-render-tree-register.ts b/packages/@glimmer/runtime/lib/debug-render-tree-register.ts
@@ -1,12 +1,3 @@
-// Side-effect-only module: registers `DebugRenderTreeImpl` as the factory
-// used by `EnvironmentImpl` when `delegate.enableDebugTooling` is true.
-//
-// Importing this file pulls `./debug-render-tree` into the bundle. Apps
-// that don't need render-tree introspection (anything not running the
-// Ember Inspector) shouldn't import this module — `EnvironmentImpl`'s
-// `debugRenderTree` will silently stay `undefined`, which is the same
-// behavior you'd get with `enableDebugTooling: false`.
-
 import DebugRenderTreeImpl from './debug-render-tree';
 import { registerDebugRenderTreeFactory } from './environment';
 
diff --git a/packages/@glimmer/runtime/lib/debug-render-tree.ts b/packages/@glimmer/runtime/lib/debug-render-tree.ts
@@ -197,7 +197,4 @@ export default class DebugRenderTreeImpl<
   }
 }
 
-// `getDebugName` now lives in `./get-debug-name` so opcodes can pull it
-// in without dragging in the full `DebugRenderTreeImpl` class. Re-export
-// for back-compat with any external consumers.
 export { getDebugName } from './get-debug-name';
diff --git a/packages/@glimmer/runtime/lib/environment.ts b/packages/@glimmer/runtime/lib/environment.ts
@@ -23,11 +23,6 @@ import { UPDATE_TAG as updateTag } from '@glimmer/validator/lib/validators';
 import { DOMChangesImpl, DOMTreeConstruction } from './dom/helper';
 import { isArgumentError } from './vm/arguments';
 
-// Lazy registration: the DebugRenderTree implementation registers itself
-// here on import (`./debug-render-tree-register`). Apps that don't need
-// render-tree introspection (anything not running the Ember Inspector)
-// won't pull `./debug-render-tree` into the bundle, since this module no
-// longer references it statically.
 type DebugRenderTreeFactory = () => DebugRenderTree;
 let debugRenderTreeFactory: DebugRenderTreeFactory | null = null;
 
diff --git a/packages/@glimmer/runtime/lib/get-debug-name.ts b/packages/@glimmer/runtime/lib/get-debug-name.ts
@@ -1,10 +1,5 @@
 import type { ComponentDefinition } from '@glimmer/interfaces';
 
-// Tiny helper extracted from `./debug-render-tree` so opcodes that need a
-// debug name don't have to import the whole `DebugRenderTreeImpl` class
-// just to reach this 4-line function. Keeps `./debug-render-tree` out of
-// bundles that aren't doing render-tree introspection (the class itself
-// is only pulled in when `debug-render-tree-register` is imported).
 export function getDebugName(
   definition: ComponentDefinition,
   manager = definition.manager

Original file line number	Diff line number	Diff line change
`@@ -1680,13 +1680,4 @@ class Component<S = unknown>`
`1680`	`1680`	`}`
`1681`	`1681`	`}`
`1682`	`1682`
`1683`		-// `Component.reopenClass({ positionalParams: [] })` and the curly component
`1684`		`-// manager registration that used to live here now happen in`
`1685`		-// `./register-curly-component`, imported as a side effect by classic-app
`1686`		-// setup (`./setup-registry`). Splitting the side effect out lets bundlers
`1687`		-// tree-shake the classic `Component` class out of bundles that don't
`1688`		`-// otherwise need it — e.g. the hello-world app that only uses`
`1689`		-// `setComponentManager` / `capabilities` from `@ember/component` via
`1690`		-// `@glimmer/component`.
`1691`		`-`
`1692`	`1683`	`export default Component;`