From f448415fce5a3eb27144e58b5aa63a094535c03a Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:46:09 -0400 Subject: [PATCH 1/2] Add plugin that declares side-effects Update sideEffects list Declare that we don't do propertyReadSideEffects --- package.json | 151 +++++++++++++++++++++++++++++++++++++++++++++- rollup.config.mjs | 93 +++++++++++++++++++++++++++- 2 files changed, 242 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 1dc3e7664b1..8ff2f7eb620 100644 --- a/package.json +++ b/package.json @@ -366,5 +366,154 @@ ] } }, - "packageManager": "pnpm@10.33.2" + "packageManager": "pnpm@10.33.2", + "sideEffects": [ + "./dist/dev/packages/@ember/-internals/container/index.js", + "./dist/dev/packages/@ember/-internals/deprecations/index.js", + "./dist/dev/packages/@ember/-internals/runtime/lib/ext/rsvp.js", + "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/-proxy.js", + "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/action_handler.js", + "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/comparable.js", + "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/container_proxy.js", + "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/registry_proxy.js", + "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/target_action_support.js", + "./dist/dev/packages/@ember/-internals/string/index.js", + "./dist/dev/packages/@ember/-internals/views/lib/compat/fallback-view-registry.js", + "./dist/dev/packages/@ember/-internals/views/lib/mixins/action_support.js", + "./dist/dev/packages/@ember/-internals/views/lib/views/core_view.js", + "./dist/dev/packages/@ember/application/index.js", + "./dist/dev/packages/@ember/application/namespace.js", + "./dist/dev/packages/@ember/array/index.js", + "./dist/dev/packages/@ember/array/proxy.js", + "./dist/dev/packages/@ember/canary-features/index.js", + "./dist/dev/packages/@ember/component/index.js", + "./dist/dev/packages/@ember/controller/index.js", + "./dist/dev/packages/@ember/debug/index.js", + "./dist/dev/packages/@ember/debug/lib/deprecate.js", + "./dist/dev/packages/@ember/debug/lib/warn.js", + "./dist/dev/packages/@ember/engine/index.js", + "./dist/dev/packages/@ember/engine/instance.js", + "./dist/dev/packages/@ember/enumerable/index.js", + "./dist/dev/packages/@ember/enumerable/mutable.js", + "./dist/dev/packages/@ember/object/-internals.js", + "./dist/dev/packages/@ember/object/compat.js", + "./dist/dev/packages/@ember/object/evented.js", + "./dist/dev/packages/@ember/object/index.js", + "./dist/dev/packages/@ember/object/mixin.js", + "./dist/dev/packages/@ember/object/observable.js", + "./dist/dev/packages/@ember/object/promise-proxy-mixin.js", + "./dist/dev/packages/@ember/object/proxy.js", + "./dist/dev/packages/@ember/reactive/collections.js", + "./dist/dev/packages/@ember/routing/index.js", + "./dist/dev/packages/@ember/routing/lib/routing-service.js", + "./dist/dev/packages/@ember/routing/none-location.js", + "./dist/dev/packages/@ember/routing/route.js", + "./dist/dev/packages/@ember/routing/router-service.js", + "./dist/dev/packages/@ember/routing/router.js", + "./dist/dev/packages/@ember/runloop/index.js", + "./dist/dev/packages/@ember/template-compiler/lib/dasherize-component-name.js", + "./dist/dev/packages/@glimmer/program/index.js", + "./dist/dev/packages/@glimmer/validator/index.js", + "./dist/dev/packages/ember-template-compiler/index.js", + "./dist/dev/packages/ember-testing/index.js", + "./dist/dev/packages/ember-testing/lib/adapters/adapter.js", + "./dist/dev/packages/rsvp/index.js", + "./dist/dev/packages/shared-chunks/arguments-gkFKpbTy.js", + "./dist/dev/packages/shared-chunks/cache-DqwW4PZn.js", + "./dist/dev/packages/shared-chunks/compiler-QUSl_urU.js", + "./dist/dev/packages/shared-chunks/container-BzzHmCNj.js", + "./dist/dev/packages/shared-chunks/curly-5lnWjV_e.js", + "./dist/dev/packages/shared-chunks/element-Djb0WF7r.js", + "./dist/dev/packages/shared-chunks/env-g-kaAFLN.js", + "./dist/dev/packages/shared-chunks/get-BnKnpMTb.js", + "./dist/dev/packages/shared-chunks/guid-Cbq2sNV_.js", + "./dist/dev/packages/shared-chunks/hash-CBwipd1d.js", + "./dist/dev/packages/shared-chunks/helper-CWl5B-4E.js", + "./dist/dev/packages/shared-chunks/index-D3YEDflV.js", + "./dist/dev/packages/shared-chunks/index-DWdOlnvT.js", + "./dist/dev/packages/shared-chunks/libraries-CaP2cp6J.js", + "./dist/dev/packages/shared-chunks/not-DuTrBRNw.js", + "./dist/dev/packages/shared-chunks/on-C42k4J29.js", + "./dist/dev/packages/shared-chunks/property_get-Dc2Ve_Hk.js", + "./dist/dev/packages/shared-chunks/reference-DElZaLK7.js", + "./dist/dev/packages/shared-chunks/render-BxsZ3FBD.js", + "./dist/dev/packages/shared-chunks/rsvp.es-DR7yS7qg.js", + "./dist/dev/packages/shared-chunks/setup-registry-CSXNMEco.js", + "./dist/dev/packages/shared-chunks/super-BBBjgF69.js", + "./dist/dev/packages/shared-chunks/template-only-fcrCerCr.js", + "./dist/dev/packages/shared-chunks/textarea-DvjbcJjV.js", + "./dist/dev/packages/shared-chunks/tracked-CzOGbr6n.js", + "./dist/dev/packages/shared-chunks/unique-id-BhLq21xA.js", + "./dist/dev/packages/shared-chunks/unrecognized-url-error-BJHjL30Q.js", + "./dist/prod/packages/@ember/-internals/container/index.js", + "./dist/prod/packages/@ember/-internals/deprecations/index.js", + "./dist/prod/packages/@ember/-internals/runtime/lib/ext/rsvp.js", + "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/-proxy.js", + "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/action_handler.js", + "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/comparable.js", + "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/container_proxy.js", + "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/registry_proxy.js", + "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/target_action_support.js", + "./dist/prod/packages/@ember/-internals/string/index.js", + "./dist/prod/packages/@ember/-internals/views/lib/compat/fallback-view-registry.js", + "./dist/prod/packages/@ember/-internals/views/lib/mixins/action_support.js", + "./dist/prod/packages/@ember/-internals/views/lib/views/core_view.js", + "./dist/prod/packages/@ember/application/index.js", + "./dist/prod/packages/@ember/application/namespace.js", + "./dist/prod/packages/@ember/array/index.js", + "./dist/prod/packages/@ember/array/proxy.js", + "./dist/prod/packages/@ember/canary-features/index.js", + "./dist/prod/packages/@ember/component/index.js", + "./dist/prod/packages/@ember/controller/index.js", + "./dist/prod/packages/@ember/engine/index.js", + "./dist/prod/packages/@ember/engine/instance.js", + "./dist/prod/packages/@ember/enumerable/index.js", + "./dist/prod/packages/@ember/enumerable/mutable.js", + "./dist/prod/packages/@ember/object/compat.js", + "./dist/prod/packages/@ember/object/evented.js", + "./dist/prod/packages/@ember/object/index.js", + "./dist/prod/packages/@ember/object/observable.js", + "./dist/prod/packages/@ember/object/promise-proxy-mixin.js", + "./dist/prod/packages/@ember/object/proxy.js", + "./dist/prod/packages/@ember/reactive/collections.js", + "./dist/prod/packages/@ember/routing/index.js", + "./dist/prod/packages/@ember/routing/lib/routing-service.js", + "./dist/prod/packages/@ember/routing/none-location.js", + "./dist/prod/packages/@ember/routing/route.js", + "./dist/prod/packages/@ember/routing/router-service.js", + "./dist/prod/packages/@ember/routing/router.js", + "./dist/prod/packages/@ember/runloop/index.js", + "./dist/prod/packages/@ember/template-compiler/lib/dasherize-component-name.js", + "./dist/prod/packages/@glimmer/program/index.js", + "./dist/prod/packages/@glimmer/validator/index.js", + "./dist/prod/packages/ember-template-compiler/index.js", + "./dist/prod/packages/ember-testing/index.js", + "./dist/prod/packages/ember-testing/lib/adapters/adapter.js", + "./dist/prod/packages/rsvp/index.js", + "./dist/prod/packages/shared-chunks/arguments-tja1iZKE.js", + "./dist/prod/packages/shared-chunks/cache-CppWGdQ4.js", + "./dist/prod/packages/shared-chunks/compiler-lBSssiWp.js", + "./dist/prod/packages/shared-chunks/curly-BVN3BwzJ.js", + "./dist/prod/packages/shared-chunks/element-CdgK_DV6.js", + "./dist/prod/packages/shared-chunks/env-DXxsTFkM.js", + "./dist/prod/packages/shared-chunks/get-tneeUZus.js", + "./dist/prod/packages/shared-chunks/guid-Cbq2sNV_.js", + "./dist/prod/packages/shared-chunks/hash-BoYO-y9I.js", + "./dist/prod/packages/shared-chunks/helper-BquvJSix.js", + "./dist/prod/packages/shared-chunks/index-BSACfUJG.js", + "./dist/prod/packages/shared-chunks/index-DQ_rYPZ7.js", + "./dist/prod/packages/shared-chunks/libraries-CbGkth0j.js", + "./dist/prod/packages/shared-chunks/not-CBwPVTAg.js", + "./dist/prod/packages/shared-chunks/on-D8CZzVgt.js", + "./dist/prod/packages/shared-chunks/property_get-BmpDn1vT.js", + "./dist/prod/packages/shared-chunks/reference-I0E7ajs4.js", + "./dist/prod/packages/shared-chunks/render-6pHjGvYr.js", + "./dist/prod/packages/shared-chunks/rsvp.es-DR7yS7qg.js", + "./dist/prod/packages/shared-chunks/setup-registry-B66SKRUi.js", + "./dist/prod/packages/shared-chunks/super-BBBjgF69.js", + "./dist/prod/packages/shared-chunks/template-only-BC4EHX0c.js", + "./dist/prod/packages/shared-chunks/textarea-n1F8oZtK.js", + "./dist/prod/packages/shared-chunks/unique-id-DCk9gt8T.js", + "./dist/prod/packages/shared-chunks/unrecognized-url-error-EgNrnFu_.js" + ] } diff --git a/rollup.config.mjs b/rollup.config.mjs index eb8029e4a04..0e012171220 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -1,9 +1,10 @@ -import { dirname, parse, resolve, join } from 'node:path'; +import { dirname, parse, resolve, join, relative, isAbsolute } from 'node:path'; import { existsSync, readFileSync, statSync, writeFileSync } from 'node:fs'; import { createRequire } from 'node:module'; import { fileURLToPath } from 'node:url'; import glob from 'glob'; import * as resolveExports from 'resolve.exports'; +import { rollup } from 'rollup'; import { babel } from '@rollup/plugin-babel'; import sharedBabelConfig from './babel.config.mjs'; @@ -97,6 +98,8 @@ function sharedESMConfig({ input, debugMacrosMode, includePackageMeta = false }) plugins.push(packageMeta()); } + plugins.push(updateSideEffects()); + return { onLog: handleRollupWarnings, input, @@ -550,6 +553,94 @@ function packageMeta() { }; } +let cleanedSideEffects = false; +function updateSideEffects() { + let manifestPath = resolve(projectRoot, 'package.json'); + let entryId = '\0side-effect-probe-entry'; + + async function hasNoSideEffects(file) { + let bundle; + try { + bundle = await rollup({ + input: entryId, + treeshake: { + moduleSideEffects: 'no-external', + /** + * The few property accesses that remain after the above + * tree-shaking (e.g. reading Mixin.prototype.reopen) are not + * effectful, so they shouldn't force a whole file into the + * sideEffects list. + */ + propertyReadSideEffects: false, + }, + onwarn() {}, + plugins: [ + { + name: 'side-effect-probe', + resolveId(source, importer) { + if (source === entryId) return entryId; + if (importer === file) return { id: source, external: true }; + return null; + }, + load(id) { + if (id === entryId) return `import ${JSON.stringify(file)};`; + }, + }, + ], + }); + let { output } = await bundle.generate({ format: 'es' }); + return output[0].code.trim() === ''; + } finally { + if (bundle) await bundle.close(); + } + } + + return { + name: 'update-side-effects-in-package-json', + async buildStart() { + // we only want to clean once, + // but add sideEffects for each sub-config that uses this plugin + if (cleanedSideEffects) return; + let manifest = JSON.parse(readFileSync(manifestPath, 'utf8')); + + delete manifest.sideEffects; + + writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + '\n'); + cleanedSideEffects = true; + }, + async writeBundle(options) { + let outputDir = isAbsolute(options.dir) ? relative(projectRoot, options.dir) : options.dir; + let files = glob.sync(`${outputDir}/packages/**/*.js`, { + cwd: projectRoot, + nodir: true, + }); + + let withSideEffects = []; + let batchSize = 50; + + for (let i = 0; i < files.length; i += batchSize) { + let batch = files.slice(i, i + batchSize); + let pure = await Promise.all( + batch.map((file) => hasNoSideEffects(resolve(projectRoot, file))) + ); + + batch.forEach((file, index) => { + if (!pure[index]) { + withSideEffects.push('./' + file); + } + }); + } + + let manifest = JSON.parse(readFileSync(manifestPath, 'utf8')); + let fromOtherBuilds = manifest.sideEffects ?? []; + + manifest.sideEffects = fromOtherBuilds.concat(withSideEffects).toSorted(); + + writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + '\n'); + }, + }; +} + const allowedCycles = [ // external and not causing problems 'node_modules/rsvp/lib/rsvp', From 4a06c42ab37ce31c4a319028105f148e0110b059 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:20:37 -0400 Subject: [PATCH 2/2] Automatically annotate things we think are probably safe when generating the sideEffects list More Automatically annotate things we think are probably safe when generating the sideEffects list Automatically annotate things we think are probably safe when generating the sideEffects list Automatically annotate things we think are probably safe when generating the sideEffects list Automatically annotate things we think are probably safe when generating the sideEffects list --- bin/side-effect-detection/index.mjs | 379 ++++++++++++++++++ bin/why-side-effect.mjs | 44 ++ package.json | 137 +------ packages/@glimmer/validator/lib/validators.ts | 1 - rollup.config.mjs | 40 +- 5 files changed, 430 insertions(+), 171 deletions(-) create mode 100644 bin/side-effect-detection/index.mjs create mode 100644 bin/why-side-effect.mjs diff --git a/bin/side-effect-detection/index.mjs b/bin/side-effect-detection/index.mjs new file mode 100644 index 00000000000..eb0a6f7c99d --- /dev/null +++ b/bin/side-effect-detection/index.mjs @@ -0,0 +1,379 @@ +// Shared machinery for deciding which published files belong in +// package.json's `sideEffects` list. Used by the `updateSideEffects` plugin +// in rollup.config.mjs (which writes the manifest during builds) and by +// bin/why-side-effect.mjs (which explains why a file is flagged). +import { createRequire } from 'node:module'; +import { rollup } from 'rollup'; + +const require = createRequire(import.meta.url); +const { transformAsync } = require('@babel/core'); + +/** + * Rollup can't know that the classic object-model is lazy: + * - `Mixin.create()` / `SomeClass.extend()` only build definitions, they don't + * touch anything outside the values they return. + * - `.reopen()` / `.reopenClass()` mutate their receiver, but when the + * receiver is defined in the same file, dropping the whole file drops the + * mutation along with it — so the file is still safe to omit when unused. + * Reopening an *imported* value (e.g. ember-testing's RSVP/Application + * extensions) is a real cross-module side-effect and stays flagged. + * - `decorateMethodV2` / `decorateFieldV2` (decorator-transforms runtime, + * emitted into class static blocks) only mutate the class being defined. + * - descriptor factories (`computed`, `alias`, `service`, ...) only build + * descriptor objects. + * - registration helpers like `setClassicDecorator` or + * `setInternalComponentManager` associate metadata with the values they are + * given; when every argument is file-local, nobody can observe the + * association unless they import this file's bindings. + * - top-level variable declarations only initialize file-local bindings, so + * every call/new inside their initializers is droppable along with the + * binding (this covers e.g. module-level `new Cache(...)` instances, whose + * constructor names are mangled in shared chunks and can't be matched by + * name). + * - top-level class declarations likewise only define a file-local class; + * what runs at module evaluation (the `extends` expression, static blocks, + * static field initializers) is droppable along with the class. + * - a top-level expression statement whose effects all land on globals or + * file-local values (e.g. `Object.setPrototypeOf(LocalClass.prototype, + * Array.prototype)`, `LIBRARIES.registerCoreLibrary('Ember', VERSION)`) + * can't be observed from outside the file, so it is droppable. Read-only + * references to imports don't count as effects; what disqualifies a + * statement is assigning into an imported value, calling an imported + * function or a method on an imported receiver (e.g. RSVP wiring, + * cross-chunk opcode registration), or passing an imported value as the + * target of a known global mutator like `Object.assign`. + * - the same goes for bare top-level blocks (the dev-build residue of + * `if (DEBUG) { ... }`, e.g. `{ Object.seal(TargetActionSupport); }`), + * as long as nothing in them has a cross-file effect or throws. + * + * This babel plugin marks those calls as `#__PURE__` (and deletes the + * local-only statements, which annotations can't express) so the side-effect + * probe below can tree-shake through them. It only runs inside the probe, + * never on published output. + */ +export function annotatePureClassicCalls() { + const alwaysPure = new Set(['create', 'extend']); + const pureWhenLocal = new Set(['reopen', 'reopenClass']); + const pureFunctions = new Set([ + 'decorateMethodV2', + 'decorateFieldV2', + 'computed', + 'alias', + 'tracked', + 'service', + 'inject', + 'dependentKeyCompat', + // the @ember/object/computed macros all just build descriptor objects + 'and', + 'bool', + 'collect', + 'deprecatingAlias', + 'empty', + 'equal', + 'filter', + 'filterBy', + 'gt', + 'gte', + 'intersect', + 'lt', + 'lte', + 'map', + 'mapBy', + 'match', + 'max', + 'min', + 'none', + 'not', + 'notEmpty', + 'oneWay', + 'or', + 'readOnly', + 'reads', + 'setDiff', + 'sort', + 'sum', + 'union', + 'uniq', + 'uniqBy', + ]); + // helpers that associate metadata keyed by one of their arguments (the + // index in this map); when the key value is file-local, the association + // can only ever be looked up through this file's bindings, so it is + // unobservable unless the file is imported. The other arguments are merely + // stored, which is no more observable than reading them. Rollup's output + // preserves these local names even in shared chunks, so matching by name + // is reliable. + const pureWhenKeyArgIsLocal = new Map([ + ['setClassicDecorator', 0], + ['setHelperManager', 1], + ['setInternalHelperManager', 1], + ['setComponentTemplate', 1], + ['setComponentManager', 1], + ['setInternalComponentManager', 1], + ['setModifierManager', 1], + ['setInternalModifierManager', 1], + ['internalHelper', 0], + ['debugFreeze', 0], + ['setProxy', 0], + ['addListener', 0], + ['removeListener', 0], + ]); + + // functions that run their callback argument on the spot (used for + // module-eval warm-ups); the callback body is what gets judged + const invokesCallbackInline = new Set(['runInDebug', 'track']); + + function isImportBinding(binding) { + return ( + binding.path.isImportSpecifier() || + binding.path.isImportDefaultSpecifier() || + binding.path.isImportNamespaceSpecifier() + ); + } + + function receiverIsLocal(path, object) { + // a call result (e.g. `EmberObject.extend({}).reopen({})`) is a value + // created in this file + if (object.type === 'CallExpression') return true; + if (object.type !== 'Identifier') return false; + let binding = path.scope.getBinding(object.name); + return Boolean(binding) && !isImportBinding(binding); + } + + // `Object`/`Reflect` helpers that mutate their first argument + const globalMutators = new Set([ + 'assign', + 'defineProperty', + 'defineProperties', + 'setPrototypeOf', + 'freeze', + 'seal', + 'set', + 'deleteProperty', + ]); + + // walk e.g. `a.b().c` down to `a` + function baseIdentifier(node) { + let current = node; + for (;;) { + if (current.type === 'MemberExpression' || current.type === 'OptionalMemberExpression') { + current = current.object; + } else if ( + current.type === 'CallExpression' || + current.type === 'OptionalCallExpression' || + current.type === 'NewExpression' + ) { + current = current.callee; + } else { + break; + } + } + return current.type === 'Identifier' ? current : null; + } + + function isImported(path, node) { + let base = baseIdentifier(node); + if (!base) return false; + let binding = path.scope.getBinding(base.name); + return Boolean(binding) && isImportBinding(binding); + } + + function hasCrossFileEffect(statementPath) { + let found = false; + function fail(path) { + found = true; + path.stop(); + } + function checkCall(path) { + let { callee, arguments: args } = path.node; + // calls the rules above already declare pure can't be cross-file effects + if (callee.type === 'Identifier' && pureFunctions.has(callee.name)) return; + if (callee.type === 'Identifier' && pureWhenKeyArgIsLocal.has(callee.name)) { + let key = args[pureWhenKeyArgIsLocal.get(callee.name)]; + if (key && key.type !== 'SpreadElement' && !isImported(path, key)) return; + } + // these invoke their callback immediately and are otherwise inert + // (track's frame push/pop is transient and its tag is discarded), so + // the callback body — judged by the traversal below, see the Function + // visitor — is the only thing that matters + if (callee.type === 'Identifier' && invokesCallbackInline.has(callee.name)) return; + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.property.type === 'Identifier' && + alwaysPure.has(callee.property.name) + ) { + return; + } + if (isImported(path, callee)) return fail(path); + // `Object.assign(imported, ...)` mutates its argument, not its receiver + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.object.type === 'Identifier' && + (callee.object.name === 'Object' || callee.object.name === 'Reflect') && + !statementPath.scope.getBinding(callee.object.name) && + callee.property.type === 'Identifier' && + globalMutators.has(callee.property.name) && + args[0] && + isImported(path, args[0]) + ) { + return fail(path); + } + } + statementPath.traverse({ + Function(fnPath) { + // a deferred function body only runs (if ever) after module + // evaluation; an immediately-invoked one runs now and its body + // counts, as do callbacks of inline invokers like runInDebug/track + let parent = fnPath.parentPath; + let isIife = parent.isCallExpression() && parent.node.callee === fnPath.node; + let isInlineCallback = + parent.isCallExpression() && + parent.node.callee.type === 'Identifier' && + invokesCallbackInline.has(parent.node.callee.name) && + parent.node.arguments[0] === fnPath.node; + if (!isIife && !isInlineCallback) fnPath.skip(); + }, + AssignmentExpression(path) { + if (isImported(path, path.node.left)) fail(path); + }, + UpdateExpression(path) { + if (isImported(path, path.node.argument)) fail(path); + }, + UnaryExpression(path) { + if (path.node.operator === 'delete' && isImported(path, path.node.argument)) fail(path); + }, + CallExpression: checkCall, + OptionalCallExpression: checkCall, + NewExpression: checkCall, + TaggedTemplateExpression(path) { + if (isImported(path, path.node.tag)) fail(path); + }, + // a throw at module evaluation is observable even when nothing imports + // the file's bindings + ThrowStatement: fail, + }); + return found; + } + + function annotate(path) { + if (path.node.leadingComments?.some((c) => /[@#]__PURE__/.test(c.value))) return; + path.addComment('leading', '#__PURE__'); + } + + function isTopLevel(path) { + let parent = path.parentPath; + if (parent.isProgram()) return true; + return ( + (parent.isExportNamedDeclaration() || parent.isExportDefaultDeclaration()) && + parent.parentPath.isProgram() + ); + } + + // calls inside nested function bodies run later (if ever), not at module + // evaluation, so they must keep their real semantics + const annotateEvalTimeCalls = { + Function(fnPath) { + fnPath.skip(); + }, + CallExpression: annotate, + NewExpression: annotate, + }; + + return { + name: 'annotate-pure-classic-calls', + visitor: { + CallExpression(path) { + let { callee } = path.node; + if (callee.type === 'Identifier' && pureFunctions.has(callee.name)) { + annotate(path); + return; + } + if (callee.type !== 'MemberExpression' || callee.computed) return; + if (callee.property.type !== 'Identifier') return; + let method = callee.property.name; + let isPure = + alwaysPure.has(method) || + (pureWhenLocal.has(method) && receiverIsLocal(path, callee.object)); + if (isPure) annotate(path); + }, + VariableDeclaration(path) { + if (!isTopLevel(path)) return; + path.traverse(annotateEvalTimeCalls); + }, + ClassDeclaration(path) { + if (!isTopLevel(path)) return; + path.traverse(annotateEvalTimeCalls); + }, + ExpressionStatement(path) { + if (!path.parentPath.isProgram()) return; + if (hasCrossFileEffect(path)) return; + path.remove(); + }, + BlockStatement(path) { + if (!path.parentPath.isProgram()) return; + if (hasCrossFileEffect(path)) return; + path.remove(); + }, + }, + }; +} + +const entryId = '\0side-effect-probe-entry'; + +/** + * Re-bundles a built file by itself (every import externalized) and returns + * whatever code survives tree-shaking. An empty result means importing the + * file does nothing observable, i.e. it is side-effect free. + */ +export async function probeSurvivingCode(file) { + let bundle; + try { + bundle = await rollup({ + input: entryId, + treeshake: { + moduleSideEffects: 'no-external', + /** + * The few property accesses that remain after the above + * tree-shaking (e.g. reading Mixin.prototype.reopen) are not + * effectful, so they shouldn't force a whole file into the + * sideEffects list. + */ + propertyReadSideEffects: false, + }, + onwarn() {}, + plugins: [ + { + name: 'side-effect-probe', + resolveId(source, importer) { + if (source === entryId) return entryId; + if (importer === file) return { id: source, external: true }; + return null; + }, + load(id) { + if (id === entryId) return `import ${JSON.stringify(file)};`; + }, + async transform(code, id) { + if (id !== file) return null; + let result = await transformAsync(code, { + configFile: false, + babelrc: false, + plugins: [annotatePureClassicCalls], + }); + return { code: result.code, map: null }; + }, + }, + ], + }); + let { output } = await bundle.generate({ format: 'es' }); + return output[0].code; + } finally { + if (bundle) await bundle.close(); + } +} + +export async function hasNoSideEffects(file) { + let code = await probeSurvivingCode(file); + return code.trim() === ''; +} diff --git a/bin/why-side-effect.mjs b/bin/why-side-effect.mjs new file mode 100644 index 00000000000..9c9d4043219 --- /dev/null +++ b/bin/why-side-effect.mjs @@ -0,0 +1,44 @@ +/* eslint-disable no-console */ +// Explains why files are in package.json's sideEffects list: re-runs the +// same probe as the updateSideEffects rollup plugin and prints the top-level +// statements that survive tree-shaking (the roots, plus whatever they retain). +// +// Usage: +// node bin/why-side-effect.mjs # every flagged dev file +// node bin/why-side-effect.mjs ... # specific dist files +import { resolve, dirname } from 'node:path'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { createRequire } from 'node:module'; +import { probeSurvivingCode } from './side-effect-detection/index.mjs'; + +const projectRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const require = createRequire(import.meta.url); +const { parseAsync } = require('@babel/core'); + +function firstLine(code, node) { + let snippet = code.slice(node.start, node.end).split('\n')[0]; + return snippet.length > 110 ? snippet.slice(0, 110) + '…' : snippet; +} + +let files = process.argv.slice(2); +if (files.length === 0) { + let manifest = JSON.parse(readFileSync(resolve(projectRoot, 'package.json'), 'utf8')); + files = (manifest.sideEffects ?? []).filter((f) => f.startsWith('./dist/dev/')); +} + +for (let rel of files) { + let file = resolve(projectRoot, rel); + let code = await probeSurvivingCode(file); + let ast = await parseAsync(code, { configFile: false, babelrc: false }); + let survivors = []; + for (let node of ast.program.body) { + // imports and function declarations are only retained as dependencies of + // the real roots + if (node.type === 'ImportDeclaration' || node.type === 'FunctionDeclaration') continue; + if (node.type === 'ExportNamedDeclaration' && !node.declaration) continue; + survivors.push(`${node.type}: ${firstLine(code, node)}`); + } + console.log(`\n=== ${rel} (${survivors.length} survivors)`); + for (let line of survivors) console.log(' ' + line); +} diff --git a/package.json b/package.json index 8ff2f7eb620..4c323a90a63 100644 --- a/package.json +++ b/package.json @@ -368,152 +368,27 @@ }, "packageManager": "pnpm@10.33.2", "sideEffects": [ - "./dist/dev/packages/@ember/-internals/container/index.js", - "./dist/dev/packages/@ember/-internals/deprecations/index.js", "./dist/dev/packages/@ember/-internals/runtime/lib/ext/rsvp.js", - "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/-proxy.js", - "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/action_handler.js", - "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/comparable.js", - "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/container_proxy.js", - "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/registry_proxy.js", - "./dist/dev/packages/@ember/-internals/runtime/lib/mixins/target_action_support.js", - "./dist/dev/packages/@ember/-internals/string/index.js", - "./dist/dev/packages/@ember/-internals/views/lib/compat/fallback-view-registry.js", - "./dist/dev/packages/@ember/-internals/views/lib/mixins/action_support.js", - "./dist/dev/packages/@ember/-internals/views/lib/views/core_view.js", - "./dist/dev/packages/@ember/application/index.js", - "./dist/dev/packages/@ember/application/namespace.js", - "./dist/dev/packages/@ember/array/index.js", - "./dist/dev/packages/@ember/array/proxy.js", - "./dist/dev/packages/@ember/canary-features/index.js", - "./dist/dev/packages/@ember/component/index.js", - "./dist/dev/packages/@ember/controller/index.js", "./dist/dev/packages/@ember/debug/index.js", - "./dist/dev/packages/@ember/debug/lib/deprecate.js", - "./dist/dev/packages/@ember/debug/lib/warn.js", - "./dist/dev/packages/@ember/engine/index.js", - "./dist/dev/packages/@ember/engine/instance.js", - "./dist/dev/packages/@ember/enumerable/index.js", - "./dist/dev/packages/@ember/enumerable/mutable.js", - "./dist/dev/packages/@ember/object/-internals.js", - "./dist/dev/packages/@ember/object/compat.js", - "./dist/dev/packages/@ember/object/evented.js", - "./dist/dev/packages/@ember/object/index.js", - "./dist/dev/packages/@ember/object/mixin.js", - "./dist/dev/packages/@ember/object/observable.js", - "./dist/dev/packages/@ember/object/promise-proxy-mixin.js", - "./dist/dev/packages/@ember/object/proxy.js", - "./dist/dev/packages/@ember/reactive/collections.js", "./dist/dev/packages/@ember/routing/index.js", - "./dist/dev/packages/@ember/routing/lib/routing-service.js", - "./dist/dev/packages/@ember/routing/none-location.js", - "./dist/dev/packages/@ember/routing/route.js", - "./dist/dev/packages/@ember/routing/router-service.js", - "./dist/dev/packages/@ember/routing/router.js", - "./dist/dev/packages/@ember/runloop/index.js", - "./dist/dev/packages/@ember/template-compiler/lib/dasherize-component-name.js", - "./dist/dev/packages/@glimmer/program/index.js", "./dist/dev/packages/@glimmer/validator/index.js", - "./dist/dev/packages/ember-template-compiler/index.js", "./dist/dev/packages/ember-testing/index.js", - "./dist/dev/packages/ember-testing/lib/adapters/adapter.js", "./dist/dev/packages/rsvp/index.js", - "./dist/dev/packages/shared-chunks/arguments-gkFKpbTy.js", - "./dist/dev/packages/shared-chunks/cache-DqwW4PZn.js", "./dist/dev/packages/shared-chunks/compiler-QUSl_urU.js", - "./dist/dev/packages/shared-chunks/container-BzzHmCNj.js", - "./dist/dev/packages/shared-chunks/curly-5lnWjV_e.js", - "./dist/dev/packages/shared-chunks/element-Djb0WF7r.js", "./dist/dev/packages/shared-chunks/env-g-kaAFLN.js", - "./dist/dev/packages/shared-chunks/get-BnKnpMTb.js", - "./dist/dev/packages/shared-chunks/guid-Cbq2sNV_.js", - "./dist/dev/packages/shared-chunks/hash-CBwipd1d.js", - "./dist/dev/packages/shared-chunks/helper-CWl5B-4E.js", - "./dist/dev/packages/shared-chunks/index-D3YEDflV.js", - "./dist/dev/packages/shared-chunks/index-DWdOlnvT.js", - "./dist/dev/packages/shared-chunks/libraries-CaP2cp6J.js", - "./dist/dev/packages/shared-chunks/not-DuTrBRNw.js", - "./dist/dev/packages/shared-chunks/on-C42k4J29.js", - "./dist/dev/packages/shared-chunks/property_get-Dc2Ve_Hk.js", - "./dist/dev/packages/shared-chunks/reference-DElZaLK7.js", - "./dist/dev/packages/shared-chunks/render-BxsZ3FBD.js", + "./dist/dev/packages/shared-chunks/index-3f7ULWdy.js", + "./dist/dev/packages/shared-chunks/render-DpEi03h2.js", "./dist/dev/packages/shared-chunks/rsvp.es-DR7yS7qg.js", - "./dist/dev/packages/shared-chunks/setup-registry-CSXNMEco.js", - "./dist/dev/packages/shared-chunks/super-BBBjgF69.js", - "./dist/dev/packages/shared-chunks/template-only-fcrCerCr.js", - "./dist/dev/packages/shared-chunks/textarea-DvjbcJjV.js", - "./dist/dev/packages/shared-chunks/tracked-CzOGbr6n.js", - "./dist/dev/packages/shared-chunks/unique-id-BhLq21xA.js", - "./dist/dev/packages/shared-chunks/unrecognized-url-error-BJHjL30Q.js", - "./dist/prod/packages/@ember/-internals/container/index.js", - "./dist/prod/packages/@ember/-internals/deprecations/index.js", + "./dist/dev/packages/shared-chunks/textarea-BpB7ZI6A.js", "./dist/prod/packages/@ember/-internals/runtime/lib/ext/rsvp.js", - "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/-proxy.js", - "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/action_handler.js", - "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/comparable.js", - "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/container_proxy.js", - "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/registry_proxy.js", - "./dist/prod/packages/@ember/-internals/runtime/lib/mixins/target_action_support.js", - "./dist/prod/packages/@ember/-internals/string/index.js", - "./dist/prod/packages/@ember/-internals/views/lib/compat/fallback-view-registry.js", - "./dist/prod/packages/@ember/-internals/views/lib/mixins/action_support.js", - "./dist/prod/packages/@ember/-internals/views/lib/views/core_view.js", - "./dist/prod/packages/@ember/application/index.js", - "./dist/prod/packages/@ember/application/namespace.js", - "./dist/prod/packages/@ember/array/index.js", - "./dist/prod/packages/@ember/array/proxy.js", - "./dist/prod/packages/@ember/canary-features/index.js", - "./dist/prod/packages/@ember/component/index.js", - "./dist/prod/packages/@ember/controller/index.js", - "./dist/prod/packages/@ember/engine/index.js", - "./dist/prod/packages/@ember/engine/instance.js", - "./dist/prod/packages/@ember/enumerable/index.js", - "./dist/prod/packages/@ember/enumerable/mutable.js", - "./dist/prod/packages/@ember/object/compat.js", - "./dist/prod/packages/@ember/object/evented.js", - "./dist/prod/packages/@ember/object/index.js", - "./dist/prod/packages/@ember/object/observable.js", - "./dist/prod/packages/@ember/object/promise-proxy-mixin.js", - "./dist/prod/packages/@ember/object/proxy.js", - "./dist/prod/packages/@ember/reactive/collections.js", - "./dist/prod/packages/@ember/routing/index.js", - "./dist/prod/packages/@ember/routing/lib/routing-service.js", - "./dist/prod/packages/@ember/routing/none-location.js", - "./dist/prod/packages/@ember/routing/route.js", - "./dist/prod/packages/@ember/routing/router-service.js", - "./dist/prod/packages/@ember/routing/router.js", - "./dist/prod/packages/@ember/runloop/index.js", - "./dist/prod/packages/@ember/template-compiler/lib/dasherize-component-name.js", - "./dist/prod/packages/@glimmer/program/index.js", "./dist/prod/packages/@glimmer/validator/index.js", - "./dist/prod/packages/ember-template-compiler/index.js", "./dist/prod/packages/ember-testing/index.js", - "./dist/prod/packages/ember-testing/lib/adapters/adapter.js", "./dist/prod/packages/rsvp/index.js", - "./dist/prod/packages/shared-chunks/arguments-tja1iZKE.js", - "./dist/prod/packages/shared-chunks/cache-CppWGdQ4.js", "./dist/prod/packages/shared-chunks/compiler-lBSssiWp.js", - "./dist/prod/packages/shared-chunks/curly-BVN3BwzJ.js", - "./dist/prod/packages/shared-chunks/element-CdgK_DV6.js", "./dist/prod/packages/shared-chunks/env-DXxsTFkM.js", - "./dist/prod/packages/shared-chunks/get-tneeUZus.js", - "./dist/prod/packages/shared-chunks/guid-Cbq2sNV_.js", - "./dist/prod/packages/shared-chunks/hash-BoYO-y9I.js", - "./dist/prod/packages/shared-chunks/helper-BquvJSix.js", - "./dist/prod/packages/shared-chunks/index-BSACfUJG.js", - "./dist/prod/packages/shared-chunks/index-DQ_rYPZ7.js", - "./dist/prod/packages/shared-chunks/libraries-CbGkth0j.js", - "./dist/prod/packages/shared-chunks/not-CBwPVTAg.js", - "./dist/prod/packages/shared-chunks/on-D8CZzVgt.js", - "./dist/prod/packages/shared-chunks/property_get-BmpDn1vT.js", - "./dist/prod/packages/shared-chunks/reference-I0E7ajs4.js", - "./dist/prod/packages/shared-chunks/render-6pHjGvYr.js", + "./dist/prod/packages/shared-chunks/index-CkFzcVLw.js", + "./dist/prod/packages/shared-chunks/render-B_2ftFTy.js", "./dist/prod/packages/shared-chunks/rsvp.es-DR7yS7qg.js", - "./dist/prod/packages/shared-chunks/setup-registry-B66SKRUi.js", - "./dist/prod/packages/shared-chunks/super-BBBjgF69.js", - "./dist/prod/packages/shared-chunks/template-only-BC4EHX0c.js", - "./dist/prod/packages/shared-chunks/textarea-n1F8oZtK.js", - "./dist/prod/packages/shared-chunks/unique-id-DCk9gt8T.js", - "./dist/prod/packages/shared-chunks/unrecognized-url-error-EgNrnFu_.js" + "./dist/prod/packages/shared-chunks/textarea-DIQlBiue.js" ] } diff --git a/packages/@glimmer/validator/lib/validators.ts b/packages/@glimmer/validator/lib/validators.ts index a701a133e53..66c7f1ac1cb 100644 --- a/packages/@glimmer/validator/lib/validators.ts +++ b/packages/@glimmer/validator/lib/validators.ts @@ -43,7 +43,6 @@ const CONSTANT_TAG_ID: ICONSTANT_TAG_ID = 3; ////////// export const COMPUTE: TagComputeSymbol = Symbol('TAG_COMPUTE') as TagComputeSymbol; -Reflect.set(globalThis, 'COMPUTE_SYMBOL', COMPUTE); ////////// diff --git a/rollup.config.mjs b/rollup.config.mjs index 0e012171220..d702deb8033 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -4,9 +4,9 @@ import { createRequire } from 'node:module'; import { fileURLToPath } from 'node:url'; import glob from 'glob'; import * as resolveExports from 'resolve.exports'; -import { rollup } from 'rollup'; import { babel } from '@rollup/plugin-babel'; import sharedBabelConfig from './babel.config.mjs'; +import { hasNoSideEffects } from './bin/side-effect-detection/index.mjs'; // eslint-disable-next-line no-redeclare const require = createRequire(import.meta.url); @@ -556,44 +556,6 @@ function packageMeta() { let cleanedSideEffects = false; function updateSideEffects() { let manifestPath = resolve(projectRoot, 'package.json'); - let entryId = '\0side-effect-probe-entry'; - - async function hasNoSideEffects(file) { - let bundle; - try { - bundle = await rollup({ - input: entryId, - treeshake: { - moduleSideEffects: 'no-external', - /** - * The few property accesses that remain after the above - * tree-shaking (e.g. reading Mixin.prototype.reopen) are not - * effectful, so they shouldn't force a whole file into the - * sideEffects list. - */ - propertyReadSideEffects: false, - }, - onwarn() {}, - plugins: [ - { - name: 'side-effect-probe', - resolveId(source, importer) { - if (source === entryId) return entryId; - if (importer === file) return { id: source, external: true }; - return null; - }, - load(id) { - if (id === entryId) return `import ${JSON.stringify(file)};`; - }, - }, - ], - }); - let { output } = await bundle.generate({ format: 'es' }); - return output[0].code.trim() === ''; - } finally { - if (bundle) await bundle.close(); - } - } return { name: 'update-side-effects-in-package-json',