From 4c26bd81f8268f15abca585ca27048a7b7d23a54 Mon Sep 17 00:00:00 2001 From: Hubert Olender Date: Sun, 24 May 2026 15:43:39 +0200 Subject: [PATCH 1/6] refactor: remove private Comparable mixin in favor of duck-type check Remove the legacy Comparable mixin (Mixin.create) and replace its only usage in compare() with a local isComparable function that checks whether compare is a function via duck-typing. - Delete packages/@ember/-internals/runtime/lib/mixins/comparable.ts - Delete comparable_test.js - Remove Comparable export from runtime/index.ts - Clean up stale references in lib/index.js, package.json, and tree-shakability test snapshots - Add regression test for non-function compare values --- lib/index.js | 1 - package.json | 1 - packages/@ember/-internals/runtime/index.ts | 1 - .../runtime/lib/mixins/comparable.ts | 43 ------------------- .../runtime/tests/mixins/comparable_test.js | 36 ---------------- packages/@ember/utils/lib/compare.ts | 25 +++++++++-- packages/@ember/utils/tests/compare_test.js | 10 ++++- tests/node-vitest/tree-shakability.test.js | 2 - 8 files changed, 30 insertions(+), 89 deletions(-) delete mode 100644 packages/@ember/-internals/runtime/lib/mixins/comparable.ts delete mode 100644 packages/@ember/-internals/runtime/tests/mixins/comparable_test.js diff --git a/lib/index.js b/lib/index.js index 6d14188dbc1..04713aa5e06 100644 --- a/lib/index.js +++ b/lib/index.js @@ -41,7 +41,6 @@ const shim = addonV1Shim(path.join(__dirname, '..'), { './dist/dev/packages/@ember/-internals/runtime/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/comparable.js', './dist/dev/packages/@ember/-internals/string/index.js', './dist/dev/packages/@ember/-internals/utility-types/index.js', './dist/dev/packages/@ember/-internals/utils/index.js', diff --git a/package.json b/package.json index 64891b338bb..35b5c1666f7 100644 --- a/package.json +++ b/package.json @@ -189,7 +189,6 @@ "@ember/-internals/runtime/lib/ext/rsvp.js": "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", "@ember/-internals/runtime/lib/mixins/-proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js", "@ember/-internals/runtime/lib/mixins/action_handler.js": "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", - "@ember/-internals/runtime/lib/mixins/comparable.js": "ember-source/@ember/-internals/runtime/lib/mixins/comparable.js", "@ember/-internals/runtime/lib/mixins/container_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "@ember/-internals/runtime/lib/mixins/registry_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", "@ember/-internals/runtime/lib/mixins/target_action_support.js": "ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js", diff --git a/packages/@ember/-internals/runtime/index.ts b/packages/@ember/-internals/runtime/index.ts index ec028a7a2da..bfba8021ffc 100644 --- a/packages/@ember/-internals/runtime/index.ts +++ b/packages/@ember/-internals/runtime/index.ts @@ -1,6 +1,5 @@ export { default as RegistryProxyMixin } from './lib/mixins/registry_proxy'; export { default as ContainerProxyMixin } from './lib/mixins/container_proxy'; -export { default as Comparable } from './lib/mixins/comparable'; export { default as ActionHandler } from './lib/mixins/action_handler'; export { default as _ProxyMixin, contentFor as _contentFor } from './lib/mixins/-proxy'; export { default as MutableEnumerable } from '@ember/enumerable/mutable'; diff --git a/packages/@ember/-internals/runtime/lib/mixins/comparable.ts b/packages/@ember/-internals/runtime/lib/mixins/comparable.ts deleted file mode 100644 index 451fd1d4885..00000000000 --- a/packages/@ember/-internals/runtime/lib/mixins/comparable.ts +++ /dev/null @@ -1,43 +0,0 @@ -import Mixin from '@ember/object/mixin'; - -/** -@module ember -*/ - -/** - Implements some standard methods for comparing objects. Add this mixin to - any class you create that can compare its instances. - - You should implement the `compare()` method. - - @class Comparable - @namespace Ember - @since Ember 0.9 - @private -*/ -interface Comparable { - compare: ((a: unknown, b: unknown) => -1 | 0 | 1) | null; -} -const Comparable = Mixin.create({ - /** - __Required.__ You must implement this method to apply this mixin. - - Override to return the result of the comparison of the two parameters. The - compare method should return: - - - `-1` if `a < b` - - `0` if `a == b` - - `1` if `a > b` - - Default implementation raises an exception. - - @method compare - @param a {Object} the first object to compare - @param b {Object} the second object to compare - @return {Number} the result of the comparison - @private - */ - compare: null, -}); - -export default Comparable; diff --git a/packages/@ember/-internals/runtime/tests/mixins/comparable_test.js b/packages/@ember/-internals/runtime/tests/mixins/comparable_test.js deleted file mode 100644 index 14920e423b2..00000000000 --- a/packages/@ember/-internals/runtime/tests/mixins/comparable_test.js +++ /dev/null @@ -1,36 +0,0 @@ -import EmberObject, { get } from '@ember/object'; -import { compare } from '@ember/utils'; -import Comparable from '../../lib/mixins/comparable'; -import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; - -class Rectangle extends EmberObject.extend(Comparable) { - length = 0; - width = 0; - - area() { - return get(this, 'length') * get(this, 'width'); - } - - compare(a, b) { - return compare(a.area(), b.area()); - } -} - -let r1, r2; - -moduleFor( - 'Comparable', - class extends AbstractTestCase { - beforeEach() { - r1 = Rectangle.create({ length: 6, width: 12 }); - r2 = Rectangle.create({ length: 6, width: 13 }); - } - - ['@test should be comparable and return the correct result'](assert) { - assert.equal(Comparable.detect(r1), true); - assert.equal(compare(r1, r1), 0); - assert.equal(compare(r1, r2), -1); - assert.equal(compare(r2, r1), 1); - } - } -); diff --git a/packages/@ember/utils/lib/compare.ts b/packages/@ember/utils/lib/compare.ts index 26dae8d10ea..41aedf0999e 100644 --- a/packages/@ember/utils/lib/compare.ts +++ b/packages/@ember/utils/lib/compare.ts @@ -1,6 +1,5 @@ import type { TypeName } from './type-of'; import typeOf from './type-of'; -import Comparable from '@ember/-internals/runtime/lib/mixins/comparable'; import { assert } from '@ember/debug'; const TYPE_ORDER: Record = { @@ -163,10 +162,30 @@ export default function compare(v: T, w: T): Compare { } } +interface Comparable { + compare: (a: unknown, b: unknown) => -1 | 0 | 1; +} + interface ComparableConstructor { - constructor: Comparable; + constructor: { + compare: (a: unknown, b: unknown) => -1 | 0 | 1; + }; } function isComparable(value: unknown): value is Comparable & ComparableConstructor { - return Comparable.detect(value); + if (typeof value !== 'object' || value === null) { + return false; + } + + let maybeComparable = value as { + compare?: unknown; + constructor?: { + compare?: unknown; + }; + }; + + return ( + typeof maybeComparable.constructor?.compare === 'function' || + typeof maybeComparable.compare === 'function' + ); } diff --git a/packages/@ember/utils/tests/compare_test.js b/packages/@ember/utils/tests/compare_test.js index 9be6181a23f..c07be94af20 100644 --- a/packages/@ember/utils/tests/compare_test.js +++ b/packages/@ember/utils/tests/compare_test.js @@ -1,10 +1,9 @@ import { compare, typeOf } from '@ember/utils'; import EmberObject from '@ember/object'; -import { Comparable } from '@ember/-internals/runtime'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; let data = []; -let Comp = EmberObject.extend(Comparable); +let Comp = EmberObject.extend(); Comp.reopenClass({ compare(obj) { @@ -84,5 +83,12 @@ moduleFor( assert.equal(compare('b', zero), 0, 'Second item comparable - returns 0 (negated)'); assert.equal(compare('c', one), -1, 'Second item comparable - returns 1 (negated)'); } + + ['@test non-function compare does not make an object comparable'](assert) { + let obj = EmberObject.create({ compare: null }); + let other = EmberObject.create({ compare: 'not a function' }); + + assert.equal(compare(obj, other), 0, 'objects with non-function compare are not comparable'); + } } ); diff --git a/tests/node-vitest/tree-shakability.test.js b/tests/node-vitest/tree-shakability.test.js index d6220a4bc3d..b6656569dca 100644 --- a/tests/node-vitest/tree-shakability.test.js +++ b/tests/node-vitest/tree-shakability.test.js @@ -115,7 +115,6 @@ it('[dev] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", - "ember-source/@ember/-internals/runtime/lib/mixins/comparable.js", "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js", @@ -317,7 +316,6 @@ it('[prod] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", - "ember-source/@ember/-internals/runtime/lib/mixins/comparable.js", "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js", From ceffdb0c781d9ea457efe596990f1cf4c6f416b7 Mon Sep 17 00:00:00 2001 From: Hubert Olender Date: Sun, 24 May 2026 16:17:05 +0200 Subject: [PATCH 2/6] refactor(mixin): remove legacy Enumerable and MutableEnumerable marker mixins - Delete obsolete @ember/enumerable stub package (index.ts, mutable.ts, package.json, and detect-only test) - Remove Enumerable import and extension from EmberArray - Remove MutableEnumerable import and extension from MutableArray - Remove MutableEnumerable re-export from @ember/-internals/runtime - Clean up Enumerable entries in root package.json, lib/index.js, tree-shakability test, and workspace package.json dependencies - Delete mutable_enumerable_test.js, which only tested MutableEnumerable.detect BREAKING CHANGE: @ember/enumerable and @ember/enumerable/mutable are removed. Legacy Enumerable.detect and MutableEnumerable.detect checks are no longer supported. Code that needs to detect Ember array behavior should use EmberArray.detect or MutableArray.detect where appropriate. --- lib/index.js | 2 -- package.json | 2 -- packages/@ember/-internals/package.json | 1 - packages/@ember/-internals/runtime/index.ts | 1 - .../tests/mixins/mutable_enumerable_test.js | 17 -------------- packages/@ember/array/index.ts | 12 ++++------ packages/@ember/array/package.json | 1 - packages/@ember/debug/package.json | 1 - packages/@ember/enumerable/index.ts | 20 ----------------- packages/@ember/enumerable/mutable.ts | 22 ------------------- packages/@ember/enumerable/package.json | 22 ------------------- .../enumerable/tests/enumerable_test.js | 17 -------------- packages/@ember/object/package.json | 1 - packages/@ember/routing/package.json | 1 - packages/@ember/utils/package.json | 1 - packages/@ember/version/package.json | 1 - packages/ember-template-compiler/package.json | 1 - packages/ember/package.json | 1 - packages/internal-test-helpers/package.json | 1 - tests/node-vitest/tree-shakability.test.js | 4 ---- 20 files changed, 4 insertions(+), 125 deletions(-) delete mode 100644 packages/@ember/-internals/runtime/tests/mixins/mutable_enumerable_test.js delete mode 100644 packages/@ember/enumerable/index.ts delete mode 100644 packages/@ember/enumerable/mutable.ts delete mode 100644 packages/@ember/enumerable/package.json delete mode 100644 packages/@ember/enumerable/tests/enumerable_test.js diff --git a/lib/index.js b/lib/index.js index 04713aa5e06..6fe4cd121b3 100644 --- a/lib/index.js +++ b/lib/index.js @@ -77,8 +77,6 @@ const shim = addonV1Shim(path.join(__dirname, '..'), { './dist/dev/packages/@ember/engine/index.js', './dist/dev/packages/@ember/engine/instance.js', './dist/dev/packages/@ember/engine/lib/engine-parent.js', - './dist/dev/packages/@ember/enumerable/index.js', - './dist/dev/packages/@ember/enumerable/mutable.js', './dist/dev/packages/@ember/helper/index.js', './dist/dev/packages/@ember/instrumentation/index.js', './dist/dev/packages/@ember/modifier/index.js', diff --git a/package.json b/package.json index 35b5c1666f7..7d2e8c3cfa2 100644 --- a/package.json +++ b/package.json @@ -233,8 +233,6 @@ "@ember/engine/instance.js": "ember-source/@ember/engine/instance.js", "@ember/engine/lib/engine-parent.js": "ember-source/@ember/engine/lib/engine-parent.js", "@ember/engine/parent.js": "ember-source/@ember/engine/parent.js", - "@ember/enumerable/index.js": "ember-source/@ember/enumerable/index.js", - "@ember/enumerable/mutable.js": "ember-source/@ember/enumerable/mutable.js", "@ember/helper/index.js": "ember-source/@ember/helper/index.js", "@ember/instrumentation/index.js": "ember-source/@ember/instrumentation/index.js", "@ember/modifier/index.js": "ember-source/@ember/modifier/index.js", diff --git a/packages/@ember/-internals/package.json b/packages/@ember/-internals/package.json index ed45dfe6c74..4d962191f54 100644 --- a/packages/@ember/-internals/package.json +++ b/packages/@ember/-internals/package.json @@ -32,7 +32,6 @@ "@ember/debug": "workspace:*", "@ember/destroyable": "workspace:*", "@ember/engine": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/helper": "workspace:*", "@ember/instrumentation": "workspace:*", "@ember/modifier": "workspace:*", diff --git a/packages/@ember/-internals/runtime/index.ts b/packages/@ember/-internals/runtime/index.ts index bfba8021ffc..cf4fa7ab1cb 100644 --- a/packages/@ember/-internals/runtime/index.ts +++ b/packages/@ember/-internals/runtime/index.ts @@ -2,7 +2,6 @@ export { default as RegistryProxyMixin } from './lib/mixins/registry_proxy'; export { default as ContainerProxyMixin } from './lib/mixins/container_proxy'; export { default as ActionHandler } from './lib/mixins/action_handler'; export { default as _ProxyMixin, contentFor as _contentFor } from './lib/mixins/-proxy'; -export { default as MutableEnumerable } from '@ember/enumerable/mutable'; export { default as TargetActionSupport } from './lib/mixins/target_action_support'; export { default as RSVP, onerrorDefault } from './lib/ext/rsvp'; // just for side effect of extending Ember.RSVP diff --git a/packages/@ember/-internals/runtime/tests/mixins/mutable_enumerable_test.js b/packages/@ember/-internals/runtime/tests/mixins/mutable_enumerable_test.js deleted file mode 100644 index 0a8af297f24..00000000000 --- a/packages/@ember/-internals/runtime/tests/mixins/mutable_enumerable_test.js +++ /dev/null @@ -1,17 +0,0 @@ -import MutableEnumerable from '@ember/enumerable/mutable'; -import ArrayProxy from '@ember/array/proxy'; -import { A } from '@ember/array'; -import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; - -moduleFor( - 'MutableEnumerable', - class extends AbstractTestCase { - ['@test should be mixed into A()'](assert) { - assert.ok(MutableEnumerable.detect(A())); - } - - ['@test should be mixed into ArrayProxy'](assert) { - assert.ok(MutableEnumerable.detect(ArrayProxy.create())); - } - } -); diff --git a/packages/@ember/array/index.ts b/packages/@ember/array/index.ts index 48e0c1b5b1e..43433efed2b 100644 --- a/packages/@ember/array/index.ts +++ b/packages/@ember/array/index.ts @@ -14,8 +14,6 @@ import { get } from '@ember/-internals/metal/lib/property_get'; import { set } from '@ember/-internals/metal/lib/property_set'; import Mixin from '@ember/object/mixin'; import { assert } from '@ember/debug'; -import Enumerable from '@ember/enumerable'; -import MutableEnumerable from '@ember/enumerable/mutable'; import compare from '@ember/utils/lib/compare'; import typeOf from '@ember/utils/lib/type-of'; import Observable from '@ember/object/observable'; @@ -242,11 +240,10 @@ function mapBy(this: EmberArray, key: string) { primitives to use it: `length()` and `objectAt()`. @class EmberArray - @uses Enumerable @since Ember 0.9.0 @public */ -interface EmberArray extends Enumerable { +interface EmberArray { /** __Required.__ You must implement this method to apply this mixin. @@ -1199,7 +1196,7 @@ interface EmberArray extends Enumerable { */ without(value: T): NativeArray; } -const EmberArray = Mixin.create(Enumerable, { +const EmberArray = Mixin.create({ init() { this._super(...arguments); setEmberArray(this); @@ -1488,10 +1485,9 @@ const EmberArray = Mixin.create(Enumerable, { @class MutableArray @uses EmberArray - @uses MutableEnumerable @public */ -interface MutableArray extends EmberArray, MutableEnumerable { +interface MutableArray extends EmberArray { /** __Required.__ You must implement this method to apply this mixin. @@ -1747,7 +1743,7 @@ interface MutableArray extends EmberArray, MutableEnumerable { */ addObjects(objects: T[]): this; } -const MutableArray = Mixin.create(EmberArray, MutableEnumerable, { +const MutableArray = Mixin.create(EmberArray, { clear() { let len = this.length; if (len === 0) { diff --git a/packages/@ember/array/package.json b/packages/@ember/array/package.json index 8585c775652..e3a78223804 100644 --- a/packages/@ember/array/package.json +++ b/packages/@ember/array/package.json @@ -14,7 +14,6 @@ "@ember/-internals": "workspace:*", "@ember/application": "workspace:*", "@ember/debug": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/object": "workspace:*", "@ember/runloop": "workspace:*", "@ember/utils": "workspace:*", diff --git a/packages/@ember/debug/package.json b/packages/@ember/debug/package.json index 0d271bb0f79..558eda637fa 100644 --- a/packages/@ember/debug/package.json +++ b/packages/@ember/debug/package.json @@ -13,7 +13,6 @@ "@ember/application": "workspace:*", "@ember/array": "workspace:*", "@ember/engine": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/object": "workspace:*", "@ember/owner": "workspace:*", "@ember/routing": "workspace:*", diff --git a/packages/@ember/enumerable/index.ts b/packages/@ember/enumerable/index.ts deleted file mode 100644 index 2929953363b..00000000000 --- a/packages/@ember/enumerable/index.ts +++ /dev/null @@ -1,20 +0,0 @@ -import Mixin from '@ember/object/mixin'; - -/** -@module @ember/enumerable -@private -*/ - -/** - The methods in this mixin have been moved to [MutableArray](/ember/release/classes/MutableArray). This mixin has - been intentionally preserved to avoid breaking Enumerable.detect checks - until the community migrates away from them. - - @class Enumerable - @private -*/ -// eslint-disable-next-line @typescript-eslint/no-empty-object-type -interface Enumerable {} -const Enumerable = Mixin.create(); - -export default Enumerable; diff --git a/packages/@ember/enumerable/mutable.ts b/packages/@ember/enumerable/mutable.ts deleted file mode 100644 index 5f1b01182b9..00000000000 --- a/packages/@ember/enumerable/mutable.ts +++ /dev/null @@ -1,22 +0,0 @@ -import Enumerable from '@ember/enumerable'; -import Mixin from '@ember/object/mixin'; - -/** -@module ember -*/ - -/** - The methods in this mixin have been moved to MutableArray. This mixin has - been intentionally preserved to avoid breaking MutableEnumerable.detect - checks until the community migrates away from them. - - @class MutableEnumerable - @namespace Ember - @uses Enumerable - @private -*/ -// eslint-disable-next-line @typescript-eslint/no-empty-object-type -interface MutableEnumerable extends Enumerable {} -const MutableEnumerable = Mixin.create(Enumerable); - -export default MutableEnumerable; diff --git a/packages/@ember/enumerable/package.json b/packages/@ember/enumerable/package.json deleted file mode 100644 index a12c717454d..00000000000 --- a/packages/@ember/enumerable/package.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "name": "@ember/enumerable", - "private": true, - "type": "module", - "exports": { - ".": "./index.ts", - "./mutable": "./mutable.ts", - "./*": "./*.ts" - }, - "dependencies": { - "@ember/-internals": "workspace:*", - "@ember/array": "workspace:*", - "@ember/debug": "workspace:*", - "@ember/object": "workspace:*", - "@glimmer/destroyable": "workspace:*", - "@glimmer/env": "workspace:*", - "@glimmer/owner": "workspace:*", - "@glimmer/util": "workspace:*", - "@glimmer/validator": "workspace:*", - "internal-test-helpers": "workspace:*" - } -} diff --git a/packages/@ember/enumerable/tests/enumerable_test.js b/packages/@ember/enumerable/tests/enumerable_test.js deleted file mode 100644 index caf4e226f29..00000000000 --- a/packages/@ember/enumerable/tests/enumerable_test.js +++ /dev/null @@ -1,17 +0,0 @@ -import Enumerable from '@ember/enumerable'; -import ArrayProxy from '@ember/array/proxy'; -import { A } from '@ember/array'; -import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; - -moduleFor( - 'Enumerable', - class extends AbstractTestCase { - ['@test should be mixed into A()'](assert) { - assert.ok(Enumerable.detect(A())); - } - - ['@test should be mixed into ArrayProxy'](assert) { - assert.ok(Enumerable.detect(ArrayProxy.create())); - } - } -); diff --git a/packages/@ember/object/package.json b/packages/@ember/object/package.json index e0c4ce1ff36..1fdba733b9f 100644 --- a/packages/@ember/object/package.json +++ b/packages/@ember/object/package.json @@ -23,7 +23,6 @@ "@ember/application": "workspace:*", "@ember/array": "workspace:*", "@ember/debug": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/runloop": "workspace:*", "@ember/service": "workspace:*", "@ember/utils": "workspace:*", diff --git a/packages/@ember/routing/package.json b/packages/@ember/routing/package.json index 5a956191f63..b9c862884e7 100644 --- a/packages/@ember/routing/package.json +++ b/packages/@ember/routing/package.json @@ -21,7 +21,6 @@ "@ember/controller": "workspace:*", "@ember/debug": "workspace:*", "@ember/engine": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/object": "workspace:*", "@ember/owner": "workspace:*", "@ember/runloop": "workspace:*", diff --git a/packages/@ember/utils/package.json b/packages/@ember/utils/package.json index a26dab58aae..335153ebd97 100644 --- a/packages/@ember/utils/package.json +++ b/packages/@ember/utils/package.json @@ -11,7 +11,6 @@ "@ember/application": "workspace:*", "@ember/array": "workspace:*", "@ember/debug": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/object": "workspace:*", "@ember/runloop": "workspace:*", "@glimmer/destroyable": "workspace:*", diff --git a/packages/@ember/version/package.json b/packages/@ember/version/package.json index f61f55d3498..5245b961648 100644 --- a/packages/@ember/version/package.json +++ b/packages/@ember/version/package.json @@ -16,7 +16,6 @@ "@ember/debug": "workspace:*", "@ember/destroyable": "workspace:*", "@ember/engine": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/instrumentation": "workspace:*", "@ember/object": "workspace:*", "@ember/routing": "workspace:*", diff --git a/packages/ember-template-compiler/package.json b/packages/ember-template-compiler/package.json index 6bb122fbb56..d9721cc4610 100644 --- a/packages/ember-template-compiler/package.json +++ b/packages/ember-template-compiler/package.json @@ -17,7 +17,6 @@ "@ember/debug": "workspace:*", "@ember/destroyable": "workspace:*", "@ember/engine": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/instrumentation": "workspace:*", "@ember/object": "workspace:*", "@ember/routing": "workspace:*", diff --git a/packages/ember/package.json b/packages/ember/package.json index 03a84ccd1e7..e062a49a15a 100644 --- a/packages/ember/package.json +++ b/packages/ember/package.json @@ -15,7 +15,6 @@ "@ember/debug": "workspace:*", "@ember/destroyable": "workspace:*", "@ember/engine": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/helper": "workspace:*", "@ember/instrumentation": "workspace:*", "@ember/modifier": "workspace:*", diff --git a/packages/internal-test-helpers/package.json b/packages/internal-test-helpers/package.json index d09f8ef17c0..59951f342cc 100644 --- a/packages/internal-test-helpers/package.json +++ b/packages/internal-test-helpers/package.json @@ -16,7 +16,6 @@ "@ember/debug": "workspace:*", "@ember/destroyable": "workspace:*", "@ember/engine": "workspace:*", - "@ember/enumerable": "workspace:*", "@ember/instrumentation": "workspace:*", "@ember/object": "workspace:*", "@ember/owner": "workspace:*", diff --git a/tests/node-vitest/tree-shakability.test.js b/tests/node-vitest/tree-shakability.test.js index b6656569dca..226d2a140a5 100644 --- a/tests/node-vitest/tree-shakability.test.js +++ b/tests/node-vitest/tree-shakability.test.js @@ -144,8 +144,6 @@ it('[dev] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/debug/lib/warn.js", "ember-source/@ember/engine/index.js", "ember-source/@ember/engine/instance.js", - "ember-source/@ember/enumerable/index.js", - "ember-source/@ember/enumerable/mutable.js", "ember-source/@ember/helper/index.js", "ember-source/@ember/instrumentation/index.js", "ember-source/@ember/modifier/index.js", @@ -344,8 +342,6 @@ it('[prod] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/debug/lib/deprecate.js", "ember-source/@ember/engine/index.js", "ember-source/@ember/engine/instance.js", - "ember-source/@ember/enumerable/index.js", - "ember-source/@ember/enumerable/mutable.js", "ember-source/@ember/helper/index.js", "ember-source/@ember/instrumentation/index.js", "ember-source/@ember/modifier/index.js", From 63dc148cdd8c08dad9d25cd14c33fb524d7775a7 Mon Sep 17 00:00:00 2001 From: Hubert Olender Date: Sun, 24 May 2026 16:32:15 +0200 Subject: [PATCH 3/6] refactor(mixin): remove PromiseProxyMixin - Delete @ember/object/promise-proxy-mixin implementation - Remove promise-proxy-mixin package exports and entrypoints - Delete PromiseProxyMixin-specific runtime tests - Remove tracked-test case that depended on ArrayProxy.extend(PromiseProxyMixin) - Clean up docs expectations and tree-shakability entries BREAKING CHANGE: @ember/object/promise-proxy-mixin is removed. Code that used ObjectProxy.extend(PromiseProxyMixin) must migrate away from PromiseProxyMixin and manage promise state explicitly. --- lib/index.js | 1 - package.json | 1 - .../integration/components/tracked-test.js | 30 - .../tests/mixins/promise_proxy_test.js | 610 ------------------ packages/@ember/object/package.json | 1 - packages/@ember/object/promise-proxy-mixin.ts | 257 -------- tests/docs/expected.js | 2 - tests/node-vitest/tree-shakability.test.js | 2 - 8 files changed, 904 deletions(-) delete mode 100644 packages/@ember/-internals/runtime/tests/mixins/promise_proxy_test.js delete mode 100644 packages/@ember/object/promise-proxy-mixin.ts diff --git a/lib/index.js b/lib/index.js index 6fe4cd121b3..3c5dbe44517 100644 --- a/lib/index.js +++ b/lib/index.js @@ -93,7 +93,6 @@ const shim = addonV1Shim(path.join(__dirname, '..'), { './dist/dev/packages/@ember/object/mixin.js', './dist/dev/packages/@ember/object/observable.js', './dist/dev/packages/@ember/object/observers.js', - './dist/dev/packages/@ember/object/promise-proxy-mixin.js', './dist/dev/packages/@ember/object/proxy.js', './dist/dev/packages/@ember/owner/index.js', './dist/dev/packages/@ember/renderer/index.js', diff --git a/package.json b/package.json index 7d2e8c3cfa2..473b0be8563 100644 --- a/package.json +++ b/package.json @@ -250,7 +250,6 @@ "@ember/object/mixin.js": "ember-source/@ember/object/mixin.js", "@ember/object/observable.js": "ember-source/@ember/object/observable.js", "@ember/object/observers.js": "ember-source/@ember/object/observers.js", - "@ember/object/promise-proxy-mixin.js": "ember-source/@ember/object/promise-proxy-mixin.js", "@ember/object/proxy.js": "ember-source/@ember/object/proxy.js", "@ember/owner/index.js": "ember-source/@ember/owner/index.js", "@ember/reactive/collections.js": "ember-source/@ember/reactive/collections.js", diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js index 550ee0278d4..3a5ce957f69 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js @@ -1,10 +1,8 @@ import EmberObject from '@ember/object'; import { A } from '@ember/array'; import ArrayProxy from '@ember/array/proxy'; -import PromiseProxyMixin from '@ember/object/promise-proxy-mixin'; import { tracked } from '@ember/-internals/metal'; import { computed, get, set } from '@ember/object'; -import { Promise } from 'rsvp'; import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers'; import GlimmerishComponent from '../../utils/glimmerish-component'; import { Component } from '../../utils/helpers'; @@ -93,34 +91,6 @@ moduleFor( this.assertText('max jackson | max jackson'); } - '@test creating an array proxy inside a tracking context does not trigger backtracking assertion'() { - let PromiseArray = ArrayProxy.extend(PromiseProxyMixin); - - class LoaderComponent extends GlimmerishComponent { - get data() { - if (!this._data) { - this._data = PromiseArray.create({ - promise: Promise.resolve([1, 2, 3]), - }); - } - - return this._data; - } - } - - this.owner.register( - 'component:loader', - setComponentTemplate( - precompileTemplate('{{#each this.data as |item|}}{{item}}{{/each}}'), - LoaderComponent - ) - ); - - this.render(''); - - this.assertText('123'); - } - '@test creating an array proxy inside a tracking context and immediately updating its content before usage does not trigger backtracking assertion'() { class LoaderComponent extends GlimmerishComponent { get data() { diff --git a/packages/@ember/-internals/runtime/tests/mixins/promise_proxy_test.js b/packages/@ember/-internals/runtime/tests/mixins/promise_proxy_test.js deleted file mode 100644 index 99822d559fd..00000000000 --- a/packages/@ember/-internals/runtime/tests/mixins/promise_proxy_test.js +++ /dev/null @@ -1,610 +0,0 @@ -import { run } from '@ember/runloop'; -import { get } from '@ember/object'; -import ObjectProxy from '@ember/object/proxy'; -import PromiseProxyMixin from '@ember/object/promise-proxy-mixin'; -import EmberRSVP from '../../lib/ext/rsvp'; -import { onerrorDefault } from '../../lib/ext/rsvp'; -import * as RSVP from 'rsvp'; -import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; - -let ObjectPromiseProxy, proxy; - -moduleFor( - 'Ember.PromiseProxy - ObjectProxy', - class extends AbstractTestCase { - beforeEach() { - ObjectPromiseProxy = ObjectProxy.extend(PromiseProxyMixin); - } - - afterEach() { - RSVP.on('error', onerrorDefault); - if (proxy) proxy.destroy(); - proxy = undefined; - } - - ['@test present on ember namespace'](assert) { - assert.ok(PromiseProxyMixin, 'expected PromiseProxyMixin to exist'); - } - - ['@test no promise, invoking then should raise'](assert) { - proxy = ObjectPromiseProxy.create(); - - assert.throws(function () { - proxy.then( - function () { - return this; - }, - function () { - return this; - } - ); - }, new RegExp("PromiseProxy's promise must be set")); - } - - ['@test fulfillment'](assert) { - let value = { - firstName: 'stef', - lastName: 'penner', - }; - - let deferred = RSVP.defer(); - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - let didFulfillCount = 0; - let didRejectCount = 0; - - proxy.then( - () => didFulfillCount++, - () => didRejectCount++ - ); - - assert.equal(get(proxy, 'content'), undefined, 'expects the proxy to have no content'); - assert.equal(get(proxy, 'reason'), undefined, 'expects the proxy to have no reason'); - assert.equal( - get(proxy, 'isPending'), - true, - 'expects the proxy to indicate that it is loading' - ); - assert.equal( - get(proxy, 'isSettled'), - false, - 'expects the proxy to indicate that it is not settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - - assert.equal(didFulfillCount, 0, 'should not yet have been fulfilled'); - assert.equal(didRejectCount, 0, 'should not yet have been rejected'); - - run(deferred, 'resolve', value); - - assert.equal(didFulfillCount, 1, 'should have been fulfilled'); - assert.equal(didRejectCount, 0, 'should not have been rejected'); - - assert.equal(get(proxy, 'content'), value, 'expects the proxy to have content'); - assert.equal(get(proxy, 'reason'), undefined, 'expects the proxy to still have no reason'); - assert.equal( - get(proxy, 'isPending'), - false, - 'expects the proxy to indicate that it is no longer loading' - ); - assert.equal( - get(proxy, 'isSettled'), - true, - 'expects the proxy to indicate that it is settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - true, - 'expects the proxy to indicate that it is fulfilled' - ); - - run(deferred, 'resolve', value); - - assert.equal(didFulfillCount, 1, 'should still have been only fulfilled once'); - assert.equal(didRejectCount, 0, 'should still not have been rejected'); - - run(deferred, 'reject', value); - - assert.equal(didFulfillCount, 1, 'should still have been only fulfilled once'); - assert.equal(didRejectCount, 0, 'should still not have been rejected'); - - assert.equal( - get(proxy, 'content'), - value, - 'expects the proxy to have still have same content' - ); - assert.equal(get(proxy, 'reason'), undefined, 'expects the proxy still to have no reason'); - assert.equal( - get(proxy, 'isPending'), - false, - 'expects the proxy to indicate that it is no longer loading' - ); - assert.equal( - get(proxy, 'isSettled'), - true, - 'expects the proxy to indicate that it is settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - true, - 'expects the proxy to indicate that it is fulfilled' - ); - - // rest of the promise semantics are tested in directly in RSVP - } - - ['@test rejection'](assert) { - let reason = new Error('failure'); - let deferred = RSVP.defer(); - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - let didFulfillCount = 0; - let didRejectCount = 0; - - proxy.then( - () => didFulfillCount++, - () => didRejectCount++ - ); - - assert.equal(get(proxy, 'content'), undefined, 'expects the proxy to have no content'); - assert.equal(get(proxy, 'reason'), undefined, 'expects the proxy to have no reason'); - assert.equal( - get(proxy, 'isPending'), - true, - 'expects the proxy to indicate that it is loading' - ); - assert.equal( - get(proxy, 'isSettled'), - false, - 'expects the proxy to indicate that it is not settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - - assert.equal(didFulfillCount, 0, 'should not yet have been fulfilled'); - assert.equal(didRejectCount, 0, 'should not yet have been rejected'); - - run(deferred, 'reject', reason); - - assert.equal(didFulfillCount, 0, 'should not yet have been fulfilled'); - assert.equal(didRejectCount, 1, 'should have been rejected'); - - assert.equal(get(proxy, 'content'), undefined, 'expects the proxy to have no content'); - assert.equal(get(proxy, 'reason'), reason, 'expects the proxy to have a reason'); - assert.equal( - get(proxy, 'isPending'), - false, - 'expects the proxy to indicate that it is not longer loading' - ); - assert.equal( - get(proxy, 'isSettled'), - true, - 'expects the proxy to indicate that it is settled' - ); - assert.equal( - get(proxy, 'isRejected'), - true, - 'expects the proxy to indicate that it is rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - - run(deferred, 'reject', reason); - - assert.equal(didFulfillCount, 0, 'should stll not yet have been fulfilled'); - assert.equal(didRejectCount, 1, 'should still remain rejected'); - - run(deferred, 'resolve', 1); - - assert.equal(didFulfillCount, 0, 'should stll not yet have been fulfilled'); - assert.equal(didRejectCount, 1, 'should still remain rejected'); - - assert.equal(get(proxy, 'content'), undefined, 'expects the proxy to have no content'); - assert.equal(get(proxy, 'reason'), reason, 'expects the proxy to have a reason'); - assert.equal( - get(proxy, 'isPending'), - false, - 'expects the proxy to indicate that it is not longer loading' - ); - assert.equal( - get(proxy, 'isSettled'), - true, - 'expects the proxy to indicate that it is settled' - ); - assert.equal( - get(proxy, 'isRejected'), - true, - 'expects the proxy to indicate that it is rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - } - - // https://github.com/emberjs/ember.js/issues/15694 - ['@test rejection without specifying reason'](assert) { - let deferred = RSVP.defer(); - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - let didFulfillCount = 0; - let didRejectCount = 0; - - proxy.then( - () => didFulfillCount++, - () => didRejectCount++ - ); - - assert.equal(get(proxy, 'content'), undefined, 'expects the proxy to have no content'); - assert.equal(get(proxy, 'reason'), undefined, 'expects the proxy to have no reason'); - assert.equal( - get(proxy, 'isPending'), - true, - 'expects the proxy to indicate that it is loading' - ); - assert.equal( - get(proxy, 'isSettled'), - false, - 'expects the proxy to indicate that it is not settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - - assert.equal(didFulfillCount, 0, 'should not yet have been fulfilled'); - assert.equal(didRejectCount, 0, 'should not yet have been rejected'); - - run(deferred, 'reject'); - - assert.equal(didFulfillCount, 0, 'should not yet have been fulfilled'); - assert.equal(didRejectCount, 1, 'should have been rejected'); - - assert.equal(get(proxy, 'content'), undefined, 'expects the proxy to have no content'); - assert.equal(get(proxy, 'reason'), undefined, 'expects the proxy to have a reason'); - assert.equal( - get(proxy, 'isPending'), - false, - 'expects the proxy to indicate that it is not longer loading' - ); - assert.equal( - get(proxy, 'isSettled'), - true, - 'expects the proxy to indicate that it is settled' - ); - assert.equal( - get(proxy, 'isRejected'), - true, - 'expects the proxy to indicate that it is rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - } - - ["@test unhandled rejects still propagate to RSVP.on('error', ...) "](assert) { - assert.expect(1); - - RSVP.on('error', onerror); - RSVP.off('error', onerrorDefault); - - let expectedReason = new Error('failure'); - let deferred = RSVP.defer(); - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - proxy.get('promise'); - - function onerror(reason) { - assert.equal(reason, expectedReason, 'expected reason'); - } - - RSVP.on('error', onerror); - RSVP.off('error', onerrorDefault); - - run(deferred, 'reject', expectedReason); - - RSVP.on('error', onerrorDefault); - RSVP.off('error', onerror); - - run(deferred, 'reject', expectedReason); - - RSVP.on('error', onerrorDefault); - RSVP.off('error', onerror); - } - - ['@test should work with promise inheritance'](assert) { - class PromiseSubclass extends RSVP.Promise {} - - proxy = ObjectPromiseProxy.create({ - promise: new PromiseSubclass(() => {}), - }); - - assert.ok(proxy.then() instanceof PromiseSubclass, 'promise proxy respected inheritance'); - } - - ['@test should reset isFulfilled and isRejected when promise is reset'](assert) { - let deferred = EmberRSVP.defer(); - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - assert.equal( - get(proxy, 'isPending'), - true, - 'expects the proxy to indicate that it is loading' - ); - assert.equal( - get(proxy, 'isSettled'), - false, - 'expects the proxy to indicate that it is not settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - - run(deferred, 'resolve'); - - assert.equal( - get(proxy, 'isPending'), - false, - 'expects the proxy to indicate that it is no longer loading' - ); - assert.equal( - get(proxy, 'isSettled'), - true, - 'expects the proxy to indicate that it is settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - true, - 'expects the proxy to indicate that it is fulfilled' - ); - - let anotherDeferred = EmberRSVP.defer(); - proxy.set('promise', anotherDeferred.promise); - - assert.equal( - get(proxy, 'isPending'), - true, - 'expects the proxy to indicate that it is loading' - ); - assert.equal( - get(proxy, 'isSettled'), - false, - 'expects the proxy to indicate that it is not settled' - ); - assert.equal( - get(proxy, 'isRejected'), - false, - 'expects the proxy to indicate that it is not rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - - run(anotherDeferred, 'reject'); - - assert.equal( - get(proxy, 'isPending'), - false, - 'expects the proxy to indicate that it is not longer loading' - ); - assert.equal( - get(proxy, 'isSettled'), - true, - 'expects the proxy to indicate that it is settled' - ); - assert.equal( - get(proxy, 'isRejected'), - true, - 'expects the proxy to indicate that it is rejected' - ); - assert.equal( - get(proxy, 'isFulfilled'), - false, - 'expects the proxy to indicate that it is not fulfilled' - ); - } - - ['@test should have content when isFulfilled is set'](assert) { - let deferred = EmberRSVP.defer(); - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - proxy.addObserver('isFulfilled', () => assert.equal(get(proxy, 'content'), true)); - - run(deferred, 'resolve', true); - } - - ['@test should have reason when isRejected is set'](assert) { - let error = new Error('Y U REJECT?!?'); - let deferred = EmberRSVP.defer(); - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - proxy.addObserver('isRejected', () => assert.equal(get(proxy, 'reason'), error)); - - try { - run(deferred, 'reject', error); - } catch (e) { - assert.equal(e, error); - } - } - - ['@test should not error if promise is resolved after proxy has been destroyed'](assert) { - let deferred = EmberRSVP.defer(); - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - proxy.then( - () => {}, - () => {} - ); - - run(proxy, 'destroy'); - - run(deferred, 'resolve', true); - - assert.ok( - true, - 'resolving the promise after the proxy has been destroyed does not raise an error' - ); - } - - ['@test should not error if promise is rejected after proxy has been destroyed'](assert) { - let deferred = EmberRSVP.defer(); - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - proxy.then( - () => {}, - () => {} - ); - - run(proxy, 'destroy'); - - run(deferred, 'reject', 'some reason'); - - assert.ok( - true, - 'rejecting the promise after the proxy has been destroyed does not raise an error' - ); - } - - ['@test promise chain is not broken if promised is resolved after proxy has been destroyed']( - assert - ) { - let deferred = EmberRSVP.defer(); - let expectedValue = {}; - let receivedValue; - let didResolveCount = 0; - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - proxy.then( - (value) => { - receivedValue = value; - didResolveCount++; - }, - () => {} - ); - - run(proxy, 'destroy'); - - run(deferred, 'resolve', expectedValue); - - assert.equal(didResolveCount, 1, 'callback called'); - assert.equal( - receivedValue, - expectedValue, - 'passed value is the value the promise was resolved with' - ); - } - - ['@test promise chain is not broken if promised is rejected after proxy has been destroyed']( - assert - ) { - let deferred = EmberRSVP.defer(); - let expectedReason = 'some reason'; - let receivedReason; - let didRejectCount = 0; - - proxy = ObjectPromiseProxy.create({ - promise: deferred.promise, - }); - - proxy.then( - () => {}, - (reason) => { - receivedReason = reason; - didRejectCount++; - } - ); - - run(proxy, 'destroy'); - - run(deferred, 'reject', expectedReason); - - assert.equal(didRejectCount, 1, 'callback called'); - assert.equal( - receivedReason, - expectedReason, - 'passed reason is the reason the promise was rejected for' - ); - } - } -); diff --git a/packages/@ember/object/package.json b/packages/@ember/object/package.json index 1fdba733b9f..f4dba9e362d 100644 --- a/packages/@ember/object/package.json +++ b/packages/@ember/object/package.json @@ -14,7 +14,6 @@ "./computed": "./computed.ts", "./compat": "./compat.ts", "./proxy": "./proxy.ts", - "./promise-proxy-mixin": "./promise-proxy-mixin.ts", "./observers": "./observers.ts", "./*": "./*.ts" }, diff --git a/packages/@ember/object/promise-proxy-mixin.ts b/packages/@ember/object/promise-proxy-mixin.ts deleted file mode 100644 index 9571771a803..00000000000 --- a/packages/@ember/object/promise-proxy-mixin.ts +++ /dev/null @@ -1,257 +0,0 @@ -import { get } from '@ember/-internals/metal/lib/property_get'; -import setProperties from '@ember/-internals/metal/lib/set_properties'; -import computed from '@ember/-internals/metal/lib/computed'; -import Mixin from '@ember/object/mixin'; -import type { AnyFn, MethodNamesOf } from '@ember/-internals/utility-types'; -import type RSVP from 'rsvp'; -import type CoreObject from '@ember/object/core'; - -/** - @module @ember/object/promise-proxy-mixin -*/ - -function tap(proxy: PromiseProxyMixin, promise: RSVP.Promise) { - setProperties(proxy, { - isFulfilled: false, - isRejected: false, - }); - - return promise.then( - (value) => { - if ( - !(proxy as unknown as CoreObject).isDestroyed && - !(proxy as unknown as CoreObject).isDestroying - ) { - setProperties(proxy, { - content: value, - isFulfilled: true, - }); - } - return value; - }, - (reason) => { - if ( - !(proxy as unknown as CoreObject).isDestroyed && - !(proxy as unknown as CoreObject).isDestroying - ) { - setProperties(proxy, { - reason, - isRejected: true, - }); - } - throw reason; - }, - 'Ember: PromiseProxy' - ); -} - -/** - A low level mixin making ObjectProxy promise-aware. - - ```javascript - import { resolve } from 'rsvp'; - import $ from 'jquery'; - import ObjectProxy from '@ember/object/proxy'; - import PromiseProxyMixin from '@ember/object/promise-proxy-mixin'; - - let ObjectPromiseProxy = ObjectProxy.extend(PromiseProxyMixin); - - let proxy = ObjectPromiseProxy.create({ - promise: resolve($.getJSON('/some/remote/data.json')) - }); - - proxy.then(function(json){ - // the json - }, function(reason) { - // the reason why you have no json - }); - ``` - - the proxy has bindable attributes which - track the promises life cycle - - ```javascript - proxy.get('isPending') //=> true - proxy.get('isSettled') //=> false - proxy.get('isRejected') //=> false - proxy.get('isFulfilled') //=> false - ``` - - When the $.getJSON completes, and the promise is fulfilled - with json, the life cycle attributes will update accordingly. - Note that $.getJSON doesn't return an ECMA specified promise, - it is useful to wrap this with an `RSVP.resolve` so that it behaves - as a spec compliant promise. - - ```javascript - proxy.get('isPending') //=> false - proxy.get('isSettled') //=> true - proxy.get('isRejected') //=> false - proxy.get('isFulfilled') //=> true - ``` - - As the proxy is an ObjectProxy, and the json now its content, - all the json properties will be available directly from the proxy. - - ```javascript - // Assuming the following json: - { - firstName: 'Stefan', - lastName: 'Penner' - } - - // both properties will accessible on the proxy - proxy.get('firstName') //=> 'Stefan' - proxy.get('lastName') //=> 'Penner' - ``` - - @class PromiseProxyMixin - @public -*/ -interface PromiseProxyMixin { - /** - If the proxied promise is rejected this will contain the reason - provided. - - @property reason - @default null - @public - */ - reason: unknown; - - /** - Once the proxied promise has settled this will become `false`. - - @property isPending - @default true - @public - */ - readonly isPending: boolean; - /** - Once the proxied promise has settled this will become `true`. - - @property isSettled - @default false - @public - */ - readonly isSettled: boolean; - - /** - Will become `true` if the proxied promise is rejected. - - @property isRejected - @default false - @public - */ - isRejected: boolean; - /** - Will become `true` if the proxied promise is fulfilled. - - @property isFulfilled - @default false - @public - */ - isFulfilled: boolean; - - /** - The promise whose fulfillment value is being proxied by this object. - - This property must be specified upon creation, and should not be - changed once created. - - Example: - - ```javascript - import ObjectProxy from '@ember/object/proxy'; - import PromiseProxyMixin from '@ember/object/promise-proxy-mixin'; - - ObjectProxy.extend(PromiseProxyMixin).create({ - promise: - }); - ``` - - @property promise - @public - */ - promise: Promise; - - /** - An alias to the proxied promise's `then`. - - See RSVP.Promise.then. - - @method then - @param {Function} callback - @return {RSVP.Promise} - @public - */ - then: this['promise']['then']; - /** - An alias to the proxied promise's `catch`. - - See RSVP.Promise.catch. - - @method catch - @param {Function} callback - @return {RSVP.Promise} - @since 1.3.0 - @public - */ - catch: this['promise']['catch']; - /** - An alias to the proxied promise's `finally`. - - See RSVP.Promise.finally. - - @method finally - @param {Function} callback - @return {RSVP.Promise} - @since 1.3.0 - @public - */ - finally: this['promise']['finally']; -} -const PromiseProxyMixin = Mixin.create({ - reason: null, - - isPending: computed('isSettled', function () { - return !get(this, 'isSettled'); - }).readOnly(), - - isSettled: computed('isRejected', 'isFulfilled', function () { - return get(this, 'isRejected') || get(this, 'isFulfilled'); - }).readOnly(), - - isRejected: false, - - isFulfilled: false, - - promise: computed({ - get() { - throw new Error("PromiseProxy's promise must be set"); - }, - set(_key, promise: RSVP.Promise) { - return tap(this, promise); - }, - }), - - then: promiseAlias('then'), - - catch: promiseAlias('catch'), - - finally: promiseAlias('finally'), -}); - -function promiseAlias>>(name: N) { - return function (this: PromiseProxyMixin, ...args: Parameters[N]>) { - let promise = get(this, 'promise'); - - // We need this cast because `Parameters` is deferred so that it is not - // possible for TS to see it will always produce the right type. However, - // since `AnyFn` has a rest type, it is allowed. See discussion on [this - // issue](https://github.com/microsoft/TypeScript/issues/47615). - return (promise[name] as AnyFn)(...args); - }; -} - -export default PromiseProxyMixin; diff --git a/tests/docs/expected.js b/tests/docs/expected.js index 275e5cbd6d3..3c068bfbb57 100644 --- a/tests/docs/expected.js +++ b/tests/docs/expected.js @@ -576,7 +576,6 @@ module.exports = { 'Observable', 'Owner', 'Promise', - 'PromiseProxyMixin', 'RegisterOptions', 'Renderer', 'Resolver', @@ -611,7 +610,6 @@ module.exports = { '@ember/object/evented', '@ember/object/mixin', '@ember/object/observable', - '@ember/object/promise-proxy-mixin', '@ember/object/proxy', '@ember/owner', '@ember/reactive', diff --git a/tests/node-vitest/tree-shakability.test.js b/tests/node-vitest/tree-shakability.test.js index 226d2a140a5..b543bf419f0 100644 --- a/tests/node-vitest/tree-shakability.test.js +++ b/tests/node-vitest/tree-shakability.test.js @@ -161,7 +161,6 @@ it('[dev] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/object/mixin.js", "ember-source/@ember/object/observable.js", "ember-source/@ember/object/observers.js", - "ember-source/@ember/object/promise-proxy-mixin.js", "ember-source/@ember/object/proxy.js", "ember-source/@ember/reactive/collections.js", "ember-source/@ember/renderer/index.js", @@ -359,7 +358,6 @@ it('[prod] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/object/mixin.js", "ember-source/@ember/object/observable.js", "ember-source/@ember/object/observers.js", - "ember-source/@ember/object/promise-proxy-mixin.js", "ember-source/@ember/object/proxy.js", "ember-source/@ember/reactive/collections.js", "ember-source/@ember/renderer/index.js", From 441ec5f9165cee90dde547329ebbb641f8ff4566 Mon Sep 17 00:00:00 2001 From: Hubert Olender Date: Sun, 24 May 2026 21:03:06 +0200 Subject: [PATCH 4/6] refactor(mixin): inline component action support - Move TargetActionSupport#triggerAction directly onto Component - Move ActionSupport#send directly onto Component - Preserve triggerAction target/action/actionContext behavior and typings - Replace TargetActionSupport mixin tests with Component#triggerAction coverage - Remove ActionSupport and TargetActionSupport exports and entrypoints - Clean up tree-shakability expectations and stale mixin comments BREAKING CHANGE: private ActionSupport and TargetActionSupport mixin modules are removed. Component still provides send() and triggerAction() directly. --- lib/index.js | 1 - package.json | 2 - .../glimmer/lib/component-managers/curly.ts | 3 +- .../-internals/glimmer/lib/component.ts | 144 +++++++- .../components/target-action-test.js | 319 ++++++++++++++++++ packages/@ember/-internals/runtime/index.ts | 1 - .../lib/mixins/target_action_support.ts | 178 ---------- .../mixins/target_action_support_test.js | 211 ------------ packages/@ember/-internals/views/index.ts | 1 - .../views/lib/mixins/action_support.ts | 46 --- tests/node-vitest/tree-shakability.test.js | 4 - 11 files changed, 457 insertions(+), 453 deletions(-) delete mode 100644 packages/@ember/-internals/runtime/lib/mixins/target_action_support.ts delete mode 100644 packages/@ember/-internals/runtime/tests/mixins/target_action_support_test.js delete mode 100644 packages/@ember/-internals/views/lib/mixins/action_support.ts diff --git a/lib/index.js b/lib/index.js index 3c5dbe44517..4b143b76974 100644 --- a/lib/index.js +++ b/lib/index.js @@ -46,7 +46,6 @@ const shim = addonV1Shim(path.join(__dirname, '..'), { './dist/dev/packages/@ember/-internals/utils/index.js', './dist/dev/packages/@ember/-internals/views/index.js', './dist/dev/packages/@ember/-internals/views/lib/compat/attrs.js', - './dist/dev/packages/@ember/-internals/views/lib/mixins/action_support.js', './dist/dev/packages/@ember/-internals/views/lib/system/utils.js', './dist/dev/packages/@ember/-internals/views/lib/views/core_view.js', './dist/dev/packages/@ember/-internals/views/lib/views/states.js', diff --git a/package.json b/package.json index 473b0be8563..598ce4a3975 100644 --- a/package.json +++ b/package.json @@ -191,14 +191,12 @@ "@ember/-internals/runtime/lib/mixins/action_handler.js": "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", "@ember/-internals/runtime/lib/mixins/container_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "@ember/-internals/runtime/lib/mixins/registry_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", - "@ember/-internals/runtime/lib/mixins/target_action_support.js": "ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js", "@ember/-internals/string/index.js": "ember-source/@ember/-internals/string/index.js", "@ember/-internals/utility-types/index.js": "ember-source/@ember/-internals/utility-types/index.js", "@ember/-internals/utils/index.js": "ember-source/@ember/-internals/utils/index.js", "@ember/-internals/views/index.js": "ember-source/@ember/-internals/views/index.js", "@ember/-internals/views/lib/compat/attrs.js": "ember-source/@ember/-internals/views/lib/compat/attrs.js", "@ember/-internals/views/lib/compat/fallback-view-registry.js": "ember-source/@ember/-internals/views/lib/compat/fallback-view-registry.js", - "@ember/-internals/views/lib/mixins/action_support.js": "ember-source/@ember/-internals/views/lib/mixins/action_support.js", "@ember/-internals/views/lib/system/event_dispatcher.js": "ember-source/@ember/-internals/views/lib/system/event_dispatcher.js", "@ember/-internals/views/lib/system/utils.js": "ember-source/@ember/-internals/views/lib/system/utils.js", "@ember/-internals/views/lib/views/core_view.js": "ember-source/@ember/-internals/views/lib/views/core_view.js", diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index 585a9b8d63d..46910f2f440 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -252,8 +252,7 @@ export default class CurlyComponentManager /* * This hook is responsible for actually instantiating the component instance. * It also is where we perform additional bookkeeping to support legacy - * features like exposed by view mixins like ChildViewSupport, ActionSupport, - * etc. + * features like exposed by view mixins, etc. */ create( owner: Owner, diff --git a/packages/@ember/-internals/glimmer/lib/component.ts b/packages/@ember/-internals/glimmer/lib/component.ts index f61e446f829..4156e4c9724 100644 --- a/packages/@ember/-internals/glimmer/lib/component.ts +++ b/packages/@ember/-internals/glimmer/lib/component.ts @@ -4,9 +4,10 @@ import { get } from '@ember/-internals/metal/lib/property_get'; import { PROPERTY_DID_CHANGE } from '@ember/-internals/metal/lib/property_events'; import type { PropertyDidChange } from '@ember/-internals/metal/lib/property_events'; import { getOwner } from '@ember/-internals/owner'; -import TargetActionSupport from '@ember/-internals/runtime/lib/mixins/target_action_support'; import type ViewStates from '@ember/-internals/views/lib/views/states'; -import ActionSupport from '@ember/-internals/views/lib/mixins/action_support'; +import inspect from '@ember/debug/lib/inspect'; +import { context } from '@ember/-internals/environment/lib/context'; +import computed from '@ember/-internals/metal/lib/computed'; import { addChildView, getChildViews, @@ -54,6 +55,42 @@ function matches(el: Element, selector: string): boolean { return elMatches.call(el, selector); } +interface Sendable { + send(action: string, ...context: unknown[]): unknown; +} + +type TriggerActionOptions = { + action?: string; + target?: unknown; + actionContext?: unknown; +}; + +function isSendable(obj: unknown): obj is Sendable { + return obj != null && typeof obj === 'object' && typeof (obj as Sendable).send === 'function'; +} + +function getTarget(instance: { target: unknown; _target?: unknown }) { + let target = get(instance, 'target'); + if (target) { + if (typeof target === 'string') { + let value = get(instance, target); + if (value === undefined) { + value = get(context.lookup, target); + } + + return value; + } else { + return target; + } + } + + if (instance._target) { + return instance._target; + } + + return null; +} + /** @module @ember/component */ @@ -786,19 +823,15 @@ declare const SIGNATURE: unique symbol; @class Component @extends Ember.CoreView - @uses Ember.TargetActionSupport - @uses Ember.ActionSupport @public */ // This type param is used in the class, so must appear here. // eslint-disable-next-line @typescript-eslint/no-unused-vars interface Component - extends CoreView, TargetActionSupport, ActionSupport, ComponentMethods {} + extends CoreView, ComponentMethods {} class Component extends CoreView.extend( - TargetActionSupport, - ActionSupport, { // These need to be overridable via extend/create but should still // have a default. Defining them here is the best way to achieve that. @@ -808,6 +841,96 @@ class Component didUpdateAttrs() {}, willRender() {}, willUpdate() {}, + + // triggerAction support + target: null, + action: null, + actionContext: null, + + actionContextObject: computed('actionContext', function () { + let actionContext = get(this, 'actionContext'); + if (typeof actionContext === 'string') { + let value = get(this, actionContext); + if (value === undefined) { + value = get(context.lookup, actionContext); + } + return value; + } else { + return actionContext; + } + }), + + /** + Send an `action` with an `actionContext` to a `target`. + + The `action`, `target`, and `actionContext` are read from the component instance + unless they are provided in the optional `opts` argument. If `actionContext` is + omitted, the component instance is used as the default context. + + @method triggerAction + @param opts {Object} (optional, with the optional keys action, target and/or actionContext) + @return {Boolean} true if the action was sent successfully and did not return false + @private + */ + triggerAction(opts: TriggerActionOptions = {}) { + let { action, target, actionContext } = opts; + action = action || get(this, 'action'); + target = target || getTarget(this); + + if (actionContext === undefined) { + actionContext = get(this, 'actionContextObject') || this; + } + + let context = Array.isArray(actionContext) ? actionContext : [actionContext]; + + if (target && action) { + let ret; + + if (isSendable(target)) { + ret = target.send(action, ...context); + } else { + assert( + `The action '${action}' did not exist on ${target}`, + typeof (target as any)[action] === 'function' + ); + ret = (target as any)[action](...context); + } + + if (ret !== false) { + return true; + } + } + + return false; + }, + + // action sending support + send(actionName: string, ...args: unknown[]) { + assert( + `Attempted to call .send() with the action '${actionName}' on the destroyed object '${this}'.`, + !this.isDestroying && !this.isDestroyed + ); + + let action = this.actions && this.actions[actionName]; + + if (action) { + let shouldBubble = action.apply(this, args) === true; + if (!shouldBubble) { + return; + } + } + + let target = get(this, 'target'); + if (target) { + assert( + `The \`target\` for ${this} (${target}) does not have a \`send\` method`, + typeof target.send === 'function' + ); + target.send(...arguments); + } else { + assert(`${inspect(this)} had no action handler for: ${actionName}`, action); + } + }, } as ComponentMethods, { concatenatedProperties: ['attributeBindings', 'classNames', 'classNameBindings'], @@ -841,6 +964,13 @@ class Component */ declare classNames: string[]; + declare target: unknown; + declare action: string | null; + declare actionContext: unknown; + declare actionContextObject: unknown; + declare triggerAction: (opts?: TriggerActionOptions) => boolean; + declare send: (actionName: string, ...args: unknown[]) => void; + /** A list of properties of the view to apply as class names. If the property is a string value, the value of that string will be applied as a class diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/target-action-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/target-action-test.js index 84a7f2a27b0..fbdbcc8b865 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/target-action-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/target-action-test.js @@ -4,6 +4,7 @@ import { action, set } from '@ember/object'; import Mixin from '@ember/object/mixin'; import Controller from '@ember/controller'; import EmberObject from '@ember/object'; +import { context } from '@ember/-internals/environment'; import { Component } from '../../utils/helpers'; @@ -197,3 +198,321 @@ moduleFor( } } ); + +moduleFor( + 'Components test: triggerAction', + class extends RenderingTestCase { + ['@test returns false if no target or action are specified'](assert) { + let component; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction(), false); + } + + ['@test invokes target method by action name'](assert) { + assert.expect(2); + let component; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + anEvent() { + assert.ok(true, 'anEvent method was called'); + }, + }); + action = 'anEvent'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction(), true); + } + + ['@test invokes target.send when target has send()'](assert) { + assert.expect(3); + let component; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + send(evt, ctx) { + assert.equal(evt, 'anEvent', 'send() method was invoked with correct event name'); + assert.equal(ctx, component, 'send() method was invoked with correct context'); + }, + }); + action = 'anEvent'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction(), true); + } + + ['@test resolves target from property path string'](assert) { + assert.expect(2); + let component; + let myTarget = EmberObject.create({ + anEvent() { + assert.ok(true, 'anEvent method was called on resolved target'); + }, + }); + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = 'myTarget'; + myTarget = myTarget; + action = 'anEvent'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction(), true); + } + + ['@test resolves target from global path when property not found on component'](assert) { + assert.expect(2); + let originalLookup = context.lookup; + let lookup = {}; + context.lookup = lookup; + + let component; + lookup.Test = {}; + lookup.Test.targetObj = EmberObject.create({ + anEvent() { + assert.ok(true, 'anEvent method was called on global object'); + }, + }); + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = 'Test.targetObj'; + action = 'anEvent'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction(), true); + + context.lookup = originalLookup; + } + + ['@test uses actionContext object specified as a property on the component'](assert) { + assert.expect(2); + let component; + let ctx = {}; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + anEvent(c) { + assert.strictEqual(c, ctx, 'anEvent method was called with the expected context'); + }, + }); + action = 'anEvent'; + actionContext = ctx; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction(), true); + } + + ['@test resolves actionContext from property path string'](assert) { + assert.expect(2); + let component; + let aContext = {}; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + anEvent(c) { + assert.strictEqual(c, aContext, 'actionContext was resolved via property path'); + }, + }); + action = 'anEvent'; + actionContext = 'aContext'; + aContext = aContext; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction(), true); + } + + ['@test uses target specified in argument over property target'](assert) { + assert.expect(2); + let component; + let targetObj = EmberObject.create({ + anEvent() { + assert.ok(true, 'anEvent method was called on override target'); + }, + }); + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + action = 'anEvent'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction({ target: targetObj }), true); + } + + ['@test uses action specified in argument over property action'](assert) { + assert.expect(2); + let component; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + anEvent() { + assert.ok(true, 'anEvent method was called via override action'); + }, + }); + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction({ action: 'anEvent' }), true); + } + + ['@test uses actionContext specified in argument over property actionContext'](assert) { + assert.expect(2); + let component; + let ctx = {}; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + anEvent(c) { + assert.strictEqual(c, ctx, 'override actionContext was passed'); + }, + }); + action = 'anEvent'; + actionContext = 'wrong'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction({ actionContext: ctx }), true); + } + + ['@test array actionContext becomes multiple arguments'](assert) { + assert.expect(3); + let component; + let param1 = 'someParam'; + let param2 = 'someOtherParam'; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + anEvent(first, second) { + assert.strictEqual(first, param1, 'first param matches'); + assert.strictEqual(second, param2, 'second param matches'); + }, + }); + action = 'anEvent'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction({ actionContext: [param1, param2] }), true); + } + + ['@test null actionContext is passed as null'](assert) { + assert.expect(2); + let component; + + this.owner.register( + 'component:foo-bar', + class extends Component { + init() { + super.init(...arguments); + component = this; + } + target = EmberObject.create({ + anEvent(ctx) { + assert.strictEqual(ctx, null, 'null actionContext was passed'); + }, + }); + action = 'anEvent'; + } + ); + + this.render('{{foo-bar}}'); + + assert.strictEqual(component.triggerAction({ actionContext: null }), true); + } + } +); diff --git a/packages/@ember/-internals/runtime/index.ts b/packages/@ember/-internals/runtime/index.ts index cf4fa7ab1cb..09668ee89bc 100644 --- a/packages/@ember/-internals/runtime/index.ts +++ b/packages/@ember/-internals/runtime/index.ts @@ -2,6 +2,5 @@ export { default as RegistryProxyMixin } from './lib/mixins/registry_proxy'; export { default as ContainerProxyMixin } from './lib/mixins/container_proxy'; export { default as ActionHandler } from './lib/mixins/action_handler'; export { default as _ProxyMixin, contentFor as _contentFor } from './lib/mixins/-proxy'; -export { default as TargetActionSupport } from './lib/mixins/target_action_support'; export { default as RSVP, onerrorDefault } from './lib/ext/rsvp'; // just for side effect of extending Ember.RSVP diff --git a/packages/@ember/-internals/runtime/lib/mixins/target_action_support.ts b/packages/@ember/-internals/runtime/lib/mixins/target_action_support.ts deleted file mode 100644 index d5a7d79229f..00000000000 --- a/packages/@ember/-internals/runtime/lib/mixins/target_action_support.ts +++ /dev/null @@ -1,178 +0,0 @@ -/** -@module ember -*/ - -import { context } from '@ember/-internals/environment/lib/context'; -import { get } from '@ember/-internals/metal/lib/property_get'; -import computed from '@ember/-internals/metal/lib/computed'; -import Mixin from '@ember/object/mixin'; -import { assert } from '@ember/debug'; -import { DEBUG } from '@glimmer/env'; - -/** -`TargetActionSupport` is a mixin that can be included in a class -to add a `triggerAction` method with semantics similar to the Handlebars -`{{action}}` helper. In normal Ember usage, the `{{action}}` helper is -usually the best choice. This mixin is most often useful when you are -doing more complex event handling in Components. - -@class TargetActionSupport -@namespace Ember -@extends Mixin -@private -*/ -interface TargetActionSupport { - target: unknown; - action: string | null; - actionContext: unknown; - actionContextObject: unknown; - triggerAction(opts?: object): unknown; - - /** @internal */ - _target?: unknown; -} -const TargetActionSupport = Mixin.create({ - target: null, - action: null, - actionContext: null, - - actionContextObject: computed('actionContext', function () { - let actionContext = get(this, 'actionContext'); - - if (typeof actionContext === 'string') { - let value = get(this, actionContext); - if (value === undefined) { - value = get(context.lookup, actionContext); - } - return value; - } else { - return actionContext; - } - }), - - /** - The following is private and vestigial. - Send an `action` with an `actionContext` to a `target`. The action, actionContext - and target will be retrieved from properties of the object. For example: - - ```javascript - import { alias } from '@ember/object/computed'; - - App.SaveButtonView = Ember.View.extend(Ember.TargetActionSupport, { - target: alias('controller'), - action: 'save', - actionContext: alias('context'), - click() { - this.triggerAction(); // Sends the `save` action, along with the current context - // to the current controller - } - }); - ``` - - The `target`, `action`, and `actionContext` can be provided as properties of - an optional object argument to `triggerAction` as well. - - ```javascript - App.SaveButtonView = Ember.View.extend(Ember.TargetActionSupport, { - click() { - this.triggerAction({ - action: 'save', - target: this.get('controller'), - actionContext: this.get('context') - }); // Sends the `save` action, along with the current context - // to the current controller - } - }); - ``` - - The `actionContext` defaults to the object you are mixing `TargetActionSupport` into. - But `target` and `action` must be specified either as properties or with the argument - to `triggerAction`, or a combination: - - ```javascript - import { alias } from '@ember/object/computed'; - - App.SaveButtonView = Ember.View.extend(Ember.TargetActionSupport, { - target: alias('controller'), - click() { - this.triggerAction({ - action: 'save' - }); // Sends the `save` action, along with a reference to `this`, - // to the current controller - } - }); - ``` - - @method triggerAction - @param opts {Object} (optional, with the optional keys action, target and/or actionContext) - @return {Boolean} true if the action was sent successfully and did not return false - @private - */ - triggerAction(opts: { action?: string; target?: unknown; actionContext?: unknown } = {}) { - let { action, target, actionContext } = opts; - action = action || get(this, 'action'); - target = target || getTarget(this); - - if (actionContext === undefined) { - actionContext = get(this, 'actionContextObject') || this; - } - - let context = Array.isArray(actionContext) ? actionContext : [actionContext]; - - if (target && action) { - let ret; - - if (isSendable(target)) { - ret = target.send(action, ...context); - } else { - assert( - `The action '${action}' did not exist on ${target}`, - typeof (target as any)[action] === 'function' - ); - ret = (target as any)[action](...context); - } - - if (ret !== false) { - return true; - } - } - - return false; - }, -}); - -interface Sendable { - send(action: string, ...context: unknown[]): unknown; -} - -function isSendable(obj: unknown): obj is Sendable { - return obj != null && typeof obj === 'object' && typeof (obj as Sendable).send === 'function'; -} - -function getTarget(instance: TargetActionSupport) { - let target = get(instance, 'target'); - if (target) { - if (typeof target === 'string') { - let value = get(instance, target); - if (value === undefined) { - value = get(context.lookup, target); - } - - return value; - } else { - return target; - } - } - - if (instance._target) { - return instance._target; - } - - return null; -} - -if (DEBUG) { - Object.seal(TargetActionSupport); -} - -export default TargetActionSupport; diff --git a/packages/@ember/-internals/runtime/tests/mixins/target_action_support_test.js b/packages/@ember/-internals/runtime/tests/mixins/target_action_support_test.js deleted file mode 100644 index be9eb69ea7b..00000000000 --- a/packages/@ember/-internals/runtime/tests/mixins/target_action_support_test.js +++ /dev/null @@ -1,211 +0,0 @@ -import { context } from '@ember/-internals/environment'; -import EmberObject from '@ember/object'; -import TargetActionSupport from '../../lib/mixins/target_action_support'; -import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; - -let originalLookup = context.lookup; -let lookup; - -moduleFor( - 'TargetActionSupport', - class extends AbstractTestCase { - beforeEach() { - context.lookup = lookup = {}; - } - - afterEach() { - context.lookup = originalLookup; - } - - ['@test it should return false if no target or action are specified'](assert) { - assert.expect(1); - - let obj = EmberObject.extend(TargetActionSupport).create(); - - assert.ok(false === obj.triggerAction(), 'no target or action was specified'); - } - - ['@test it should support actions specified as strings'](assert) { - assert.expect(2); - - let obj = EmberObject.extend(TargetActionSupport).create({ - target: EmberObject.create({ - anEvent() { - assert.ok(true, 'anEvent method was called'); - }, - }), - - action: 'anEvent', - }); - - assert.ok(true === obj.triggerAction(), 'a valid target and action were specified'); - } - - ['@test it should invoke the send() method on objects that implement it'](assert) { - assert.expect(3); - - let obj = EmberObject.extend(TargetActionSupport).create({ - target: EmberObject.create({ - send(evt, context) { - assert.equal(evt, 'anEvent', 'send() method was invoked with correct event name'); - assert.equal(context, obj, 'send() method was invoked with correct context'); - }, - }), - - action: 'anEvent', - }); - - assert.ok(true === obj.triggerAction(), 'a valid target and action were specified'); - } - - ['@test it should find targets specified using a property path'](assert) { - assert.expect(2); - - let Test = {}; - lookup.Test = Test; - - Test.targetObj = EmberObject.create({ - anEvent() { - assert.ok(true, 'anEvent method was called on global object'); - }, - }); - - let myObj = EmberObject.extend(TargetActionSupport).create({ - target: 'Test.targetObj', - action: 'anEvent', - }); - - assert.ok(true === myObj.triggerAction(), 'a valid target and action were specified'); - } - - ['@test it should use an actionContext object specified as a property on the object'](assert) { - assert.expect(2); - let obj = EmberObject.extend(TargetActionSupport).create({ - action: 'anEvent', - actionContext: {}, - target: EmberObject.create({ - anEvent(ctx) { - assert.ok( - obj.actionContext === ctx, - 'anEvent method was called with the expected context' - ); - }, - }), - }); - assert.ok(true === obj.triggerAction(), 'a valid target and action were specified'); - } - - ['@test it should find an actionContext specified as a property path'](assert) { - assert.expect(2); - - let Test = {}; - lookup.Test = Test; - Test.aContext = {}; - - let obj = EmberObject.extend(TargetActionSupport).create({ - action: 'anEvent', - actionContext: 'Test.aContext', - target: EmberObject.create({ - anEvent(ctx) { - assert.ok(Test.aContext === ctx, 'anEvent method was called with the expected context'); - }, - }), - }); - - assert.ok(true === obj.triggerAction(), 'a valid target and action were specified'); - } - - ['@test it should use the target specified in the argument'](assert) { - assert.expect(2); - let targetObj = EmberObject.create({ - anEvent() { - assert.ok(true, 'anEvent method was called'); - }, - }); - let obj = EmberObject.extend(TargetActionSupport).create({ - action: 'anEvent', - }); - - assert.ok( - true === obj.triggerAction({ target: targetObj }), - 'a valid target and action were specified' - ); - } - - ['@test it should use the action specified in the argument'](assert) { - assert.expect(2); - - let obj = EmberObject.extend(TargetActionSupport).create({ - target: EmberObject.create({ - anEvent() { - assert.ok(true, 'anEvent method was called'); - }, - }), - }); - assert.ok( - true === obj.triggerAction({ action: 'anEvent' }), - 'a valid target and action were specified' - ); - } - - ['@test it should use the actionContext specified in the argument'](assert) { - assert.expect(2); - let context = {}; - let obj = EmberObject.extend(TargetActionSupport).create({ - target: EmberObject.create({ - anEvent(ctx) { - assert.ok(context === ctx, 'anEvent method was called with the expected context'); - }, - }), - action: 'anEvent', - }); - - assert.ok( - true === obj.triggerAction({ actionContext: context }), - 'a valid target and action were specified' - ); - } - - ['@test it should allow multiple arguments from actionContext'](assert) { - assert.expect(3); - let param1 = 'someParam'; - let param2 = 'someOtherParam'; - let obj = EmberObject.extend(TargetActionSupport).create({ - target: EmberObject.create({ - anEvent(first, second) { - assert.ok( - first === param1, - 'anEvent method was called with the expected first argument' - ); - assert.ok( - second === param2, - 'anEvent method was called with the expected second argument' - ); - }, - }), - action: 'anEvent', - }); - - assert.ok( - true === obj.triggerAction({ actionContext: [param1, param2] }), - 'a valid target and action were specified' - ); - } - - ['@test it should use a null value specified in the actionContext argument'](assert) { - assert.expect(2); - let obj = EmberObject.extend(TargetActionSupport).create({ - target: EmberObject.create({ - anEvent(ctx) { - assert.ok(null === ctx, 'anEvent method was called with the expected context (null)'); - }, - }), - action: 'anEvent', - }); - assert.ok( - true === obj.triggerAction({ actionContext: null }), - 'a valid target and action were specified' - ); - } - } -); diff --git a/packages/@ember/-internals/views/index.ts b/packages/@ember/-internals/views/index.ts index d68ea2baff8..9012539a925 100644 --- a/packages/@ember/-internals/views/index.ts +++ b/packages/@ember/-internals/views/index.ts @@ -17,6 +17,5 @@ export { } from './lib/system/utils'; export { default as EventDispatcher } from './lib/system/event_dispatcher'; export { default as CoreView } from './lib/views/core_view'; -export { default as ActionSupport } from './lib/mixins/action_support'; export { MUTABLE_CELL } from './lib/compat/attrs'; export { default as ViewStates } from './lib/views/states'; diff --git a/packages/@ember/-internals/views/lib/mixins/action_support.ts b/packages/@ember/-internals/views/lib/mixins/action_support.ts deleted file mode 100644 index a4c95d28ac3..00000000000 --- a/packages/@ember/-internals/views/lib/mixins/action_support.ts +++ /dev/null @@ -1,46 +0,0 @@ -/** - @module ember -*/ -import { get } from '@ember/-internals/metal/lib/property_get'; -import Mixin from '@ember/object/mixin'; -import inspect from '@ember/debug/lib/inspect'; -import { assert } from '@ember/debug'; - -/** - @class ActionSupport - @namespace Ember - @private -*/ -interface ActionSupport { - send(actionName: string, ...args: unknown[]): void; -} -const ActionSupport = Mixin.create({ - send(actionName: string, ...args: unknown[]) { - assert( - `Attempted to call .send() with the action '${actionName}' on the destroyed object '${this}'.`, - !this.isDestroying && !this.isDestroyed - ); - - let action = this.actions && this.actions[actionName]; - - if (action) { - let shouldBubble = action.apply(this, args) === true; - if (!shouldBubble) { - return; - } - } - - let target = get(this, 'target'); - if (target) { - assert( - `The \`target\` for ${this} (${target}) does not have a \`send\` method`, - typeof target.send === 'function' - ); - target.send(...arguments); - } else { - assert(`${inspect(this)} had no action handler for: ${actionName}`, action); - } - }, -}); - -export default ActionSupport; diff --git a/tests/node-vitest/tree-shakability.test.js b/tests/node-vitest/tree-shakability.test.js index b543bf419f0..815af53234c 100644 --- a/tests/node-vitest/tree-shakability.test.js +++ b/tests/node-vitest/tree-shakability.test.js @@ -117,11 +117,9 @@ it('[dev] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", - "ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js", "ember-source/@ember/-internals/utils/index.js", "ember-source/@ember/-internals/views/index.js", "ember-source/@ember/-internals/views/lib/compat/fallback-view-registry.js", - "ember-source/@ember/-internals/views/lib/mixins/action_support.js", "ember-source/@ember/-internals/views/lib/system/event_dispatcher.js", "ember-source/@ember/-internals/views/lib/system/utils.js", "ember-source/@ember/-internals/views/lib/views/core_view.js", @@ -315,11 +313,9 @@ it('[prod] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", - "ember-source/@ember/-internals/runtime/lib/mixins/target_action_support.js", "ember-source/@ember/-internals/utils/index.js", "ember-source/@ember/-internals/views/index.js", "ember-source/@ember/-internals/views/lib/compat/fallback-view-registry.js", - "ember-source/@ember/-internals/views/lib/mixins/action_support.js", "ember-source/@ember/-internals/views/lib/system/event_dispatcher.js", "ember-source/@ember/-internals/views/lib/system/utils.js", "ember-source/@ember/-internals/views/lib/views/core_view.js", From 25a1aaef27cce48200d09bf0b243a95c93137d08 Mon Sep 17 00:00:00 2001 From: Hubert Olender Date: Mon, 25 May 2026 14:19:07 +0200 Subject: [PATCH 5/6] refactor(mixin): remove private ProxyMixin Inline proxy behavior directly into ObjectProxy. Move shared proxy tracking helpers into runtime/lib/proxy-utils: - ContentProxy - contentFor - customTagForProxy Remove the private _ProxyMixin runtime export and delete the old runtime/lib/mixins/-proxy module. Update internal imports, package entrypoints, and tree-shakability expectations accordingly. BREAKING CHANGE: private _ProxyMixin and runtime/lib/mixins/-proxy are removed. ObjectProxy still provides content forwarding, unknownProperty, setUnknownProperty, and isTruthy directly. --- lib/index.js | 1 - package.json | 1 - .../-internals/glimmer/lib/helpers/each-in.ts | 2 +- .../-internals/metal/lib/property_get.ts | 4 +- packages/@ember/-internals/runtime/index.ts | 2 +- .../-internals/runtime/lib/mixins/-proxy.ts | 147 ------------------ .../-internals/runtime/lib/proxy-utils.ts | 56 +++++++ .../@ember/-internals/utils/lib/is_proxy.ts | 6 +- packages/@ember/object/proxy.ts | 80 ++++++++-- .../ember/tests/service_injection_test.js | 3 +- tests/node-vitest/tree-shakability.test.js | 2 - 11 files changed, 133 insertions(+), 171 deletions(-) delete mode 100644 packages/@ember/-internals/runtime/lib/mixins/-proxy.ts create mode 100644 packages/@ember/-internals/runtime/lib/proxy-utils.ts diff --git a/lib/index.js b/lib/index.js index 4b143b76974..f4d713f1412 100644 --- a/lib/index.js +++ b/lib/index.js @@ -40,7 +40,6 @@ const shim = addonV1Shim(path.join(__dirname, '..'), { './dist/dev/packages/@ember/-internals/routing/index.js', './dist/dev/packages/@ember/-internals/runtime/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/string/index.js', './dist/dev/packages/@ember/-internals/utility-types/index.js', './dist/dev/packages/@ember/-internals/utils/index.js', diff --git a/package.json b/package.json index 598ce4a3975..ee8ff41c134 100644 --- a/package.json +++ b/package.json @@ -187,7 +187,6 @@ "@ember/-internals/routing/index.js": "ember-source/@ember/-internals/routing/index.js", "@ember/-internals/runtime/index.js": "ember-source/@ember/-internals/runtime/index.js", "@ember/-internals/runtime/lib/ext/rsvp.js": "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", - "@ember/-internals/runtime/lib/mixins/-proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js", "@ember/-internals/runtime/lib/mixins/action_handler.js": "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", "@ember/-internals/runtime/lib/mixins/container_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "@ember/-internals/runtime/lib/mixins/registry_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", diff --git a/packages/@ember/-internals/glimmer/lib/helpers/each-in.ts b/packages/@ember/-internals/glimmer/lib/helpers/each-in.ts index 1e4c51bc083..f6ac58b4d08 100644 --- a/packages/@ember/-internals/glimmer/lib/helpers/each-in.ts +++ b/packages/@ember/-internals/glimmer/lib/helpers/each-in.ts @@ -2,7 +2,7 @@ @module ember */ import { tagForObject } from '@ember/-internals/metal/lib/tags'; -import { contentFor as _contentFor } from '@ember/-internals/runtime/lib/mixins/-proxy'; +import { contentFor as _contentFor } from '@ember/-internals/runtime/lib/proxy-utils'; import { isProxy } from '@ember/-internals/utils/lib/is_proxy'; import { assert } from '@ember/debug'; import type { CapturedArguments } from '@glimmer/interfaces'; diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index c1cd6eb2168..64eaf43b994 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/packages/@ember/-internals/metal/lib/property_get.ts @@ -1,7 +1,7 @@ /** @module @ember/object */ -import type ProxyMixin from '@ember/-internals/runtime/lib/mixins/-proxy'; +import type { ContentProxy } from '@ember/-internals/runtime/lib/proxy-utils'; import { setProxy } from '@ember/-internals/utils/lib/is_proxy'; import { isEmberArray } from '@ember/array/-internals'; import { assert } from '@ember/debug'; @@ -168,7 +168,7 @@ _getProp({ unknownProperty() {} }, 1 as any); get({}, 'foo'); get({}, 'foo.bar'); -let fakeProxy = {} as ProxyMixin; +let fakeProxy = {} as ContentProxy; setProxy(fakeProxy); track(() => _getProp({}, 'a')); diff --git a/packages/@ember/-internals/runtime/index.ts b/packages/@ember/-internals/runtime/index.ts index 09668ee89bc..b0228b1dc12 100644 --- a/packages/@ember/-internals/runtime/index.ts +++ b/packages/@ember/-internals/runtime/index.ts @@ -1,6 +1,6 @@ export { default as RegistryProxyMixin } from './lib/mixins/registry_proxy'; export { default as ContainerProxyMixin } from './lib/mixins/container_proxy'; export { default as ActionHandler } from './lib/mixins/action_handler'; -export { default as _ProxyMixin, contentFor as _contentFor } from './lib/mixins/-proxy'; +export { contentFor as _contentFor } from './lib/proxy-utils'; export { default as RSVP, onerrorDefault } from './lib/ext/rsvp'; // just for side effect of extending Ember.RSVP diff --git a/packages/@ember/-internals/runtime/lib/mixins/-proxy.ts b/packages/@ember/-internals/runtime/lib/mixins/-proxy.ts deleted file mode 100644 index ddb31a6c18d..00000000000 --- a/packages/@ember/-internals/runtime/lib/mixins/-proxy.ts +++ /dev/null @@ -1,147 +0,0 @@ -/** -@module ember -*/ - -import { meta } from '@ember/-internals/meta/lib/meta'; -import Mixin from '@ember/object/mixin'; -import { get } from '@ember/-internals/metal/lib/property_get'; -import { set } from '@ember/-internals/metal/lib/property_set'; -import { defineProperty } from '@ember/-internals/metal/lib/properties'; -import { tagForObject, tagForProperty } from '@ember/-internals/metal/lib/tags'; -import computed from '@ember/-internals/metal/lib/computed'; -import { setProxy, isProxy } from '@ember/-internals/utils/lib/is_proxy'; -import { setupMandatorySetter } from '@ember/-internals/utils/lib/mandatory-setter'; -import { isObject } from '@ember/-internals/utils/lib/spec'; -import { assert } from '@ember/debug'; -import { DEBUG } from '@glimmer/env'; -import { setCustomTagFor } from '@glimmer/manager/lib/util/args-proxy'; -import type { UpdatableTag, Tag } from '@glimmer/interfaces'; -import { combine, UPDATE_TAG as updateTag } from '@glimmer/validator/lib/validators'; -import { tagFor, tagMetaFor } from '@glimmer/validator/lib/meta'; - -export function contentFor(proxy: ProxyMixin): T | null { - let content = get(proxy, 'content'); - // SAFETY: Ideally we'd assert instead of casting, but @glimmer/validator doesn't give us - // sufficient public types for this. Previously this code was .js and worked correctly so - // hopefully this is sufficiently reliable. - updateTag(tagForObject(proxy) as UpdatableTag, tagForObject(content)); - return content; -} - -function customTagForProxy(proxy: object, key: string, addMandatorySetter?: boolean): Tag { - assert('Expected a proxy', isProxy(proxy)); - - let meta = tagMetaFor(proxy); - let tag = tagFor(proxy, key, meta); - - if (DEBUG) { - // TODO: Replace this with something more first class for tracking tags in DEBUG - // SAFETY: This is not an officially supported property but setting shouldn't cause issues. - (tag as any)._propertyKey = key; - } - - if (key in proxy) { - if (DEBUG && addMandatorySetter) { - assert('[BUG] setupMandatorySetter should be set when debugging', setupMandatorySetter); - setupMandatorySetter(tag, proxy, key); - } - - return tag; - } else { - let tags: Tag[] = [tag, tagFor(proxy, 'content', meta)]; - - let content = contentFor(proxy); - - if (isObject(content)) { - tags.push(tagForProperty(content, key, addMandatorySetter)); - } - - return combine(tags); - } -} - -/** - `ProxyMixin` forwards all properties not defined by the proxy itself - to a proxied `content` object. See ObjectProxy for more details. - - @class ProxyMixin - @namespace Ember - @private -*/ -interface ProxyMixin { - /** - The object whose properties will be forwarded. - - @property content - @type {unknown} - @default null - @public - */ - content: T | null; - - willDestroy(): void; - - isTruthy: boolean; - - unknownProperty(key: K): T[K] | undefined; - unknownProperty(key: string): unknown; - - setUnknownProperty(key: K, value: T[K]): T[K]; - setUnknownProperty(key: string, value: V): V; -} - -const ProxyMixin = Mixin.create({ - /** - The object whose properties will be forwarded. - - @property content - @type {unknown} - @default null - @public - */ - content: null, - - init() { - this._super(...arguments); - setProxy(this); - tagForObject(this); - setCustomTagFor(this, customTagForProxy); - }, - - willDestroy() { - this.set('content', null); - this._super(...arguments); - }, - - isTruthy: computed('content', function () { - return Boolean(get(this, 'content')); - }), - - unknownProperty(key: string) { - let content = contentFor(this); - return content ? get(content, key) : undefined; - }, - - setUnknownProperty(key: string, value: unknown) { - let m = meta(this); - - if (m.isInitializing() || m.isPrototypeMeta(this)) { - // if marked as prototype or object is initializing then just - // defineProperty rather than delegate - defineProperty(this, key, null, value); - return value; - } - - let content = contentFor(this); - - assert( - `Cannot delegate set('${key}', ${value}) to the 'content' property of object proxy ${this}: its 'content' is undefined.`, - content - ); - - // SAFETY: We don't actually guarantee that this is an object, so this isn't necessarily safe :( - return set(content as object, key, value); - }, -}); - -export default ProxyMixin; diff --git a/packages/@ember/-internals/runtime/lib/proxy-utils.ts b/packages/@ember/-internals/runtime/lib/proxy-utils.ts new file mode 100644 index 00000000000..887a364bbc6 --- /dev/null +++ b/packages/@ember/-internals/runtime/lib/proxy-utils.ts @@ -0,0 +1,56 @@ +import { get } from '@ember/-internals/metal/lib/property_get'; +import { tagForProperty } from '@ember/-internals/metal/lib/tags'; +import { tagForObject } from '@ember/-internals/metal/lib/tags'; +import { isProxy } from '@ember/-internals/utils/lib/is_proxy'; +import { setupMandatorySetter } from '@ember/-internals/utils/lib/mandatory-setter'; +import { isObject } from '@ember/-internals/utils/lib/spec'; +import { assert } from '@ember/debug'; +import { DEBUG } from '@glimmer/env'; +import type { UpdatableTag, Tag } from '@glimmer/interfaces'; +import { combine, UPDATE_TAG as updateTag } from '@glimmer/validator/lib/validators'; +import { tagFor, tagMetaFor } from '@glimmer/validator/lib/meta'; + +export interface ContentProxy { + content: T | null; +} + +export function contentFor(proxy: ContentProxy): T | null { + let content = get(proxy, 'content'); + // SAFETY: Ideally we'd assert instead of casting, but @glimmer/validator doesn't give us + // sufficient public types for this. Previously this code was .js and worked correctly so + // hopefully this is sufficiently reliable. + updateTag(tagForObject(proxy) as UpdatableTag, tagForObject(content)); + return content; +} + +export function customTagForProxy(proxy: object, key: string, addMandatorySetter?: boolean): Tag { + assert('Expected a proxy', isProxy(proxy)); + + let m = tagMetaFor(proxy); + let tag = tagFor(proxy, key, m); + + if (DEBUG) { + // TODO: Replace this with something more first class for tracking tags in DEBUG + // SAFETY: This is not an officially supported property but setting shouldn't cause issues. + (tag as any)._propertyKey = key; + } + + if (key in proxy) { + if (DEBUG && addMandatorySetter) { + assert('[BUG] setupMandatorySetter should be set when debugging', setupMandatorySetter); + setupMandatorySetter(tag, proxy, key); + } + + return tag; + } else { + let tags: Tag[] = [tag, tagFor(proxy, 'content', m)]; + + let content = contentFor(proxy); + + if (isObject(content)) { + tags.push(tagForProperty(content, key, addMandatorySetter)); + } + + return combine(tags); + } +} diff --git a/packages/@ember/-internals/utils/lib/is_proxy.ts b/packages/@ember/-internals/utils/lib/is_proxy.ts index 443e4c805b0..9eabbfcab89 100644 --- a/packages/@ember/-internals/utils/lib/is_proxy.ts +++ b/packages/@ember/-internals/utils/lib/is_proxy.ts @@ -1,16 +1,16 @@ -import type ProxyMixin from '@ember/-internals/runtime/lib/mixins/-proxy'; +import type { ContentProxy } from '@ember/-internals/runtime/lib/proxy-utils'; import { isObject } from './spec'; const PROXIES = new WeakSet(); -export function isProxy(value: unknown): value is ProxyMixin { +export function isProxy(value: unknown): value is ContentProxy { if (isObject(value)) { return PROXIES.has(value); } return false; } -export function setProxy(object: ProxyMixin): void { +export function setProxy(object: ContentProxy): void { if (isObject(object)) { PROXIES.add(object); } diff --git a/packages/@ember/object/proxy.ts b/packages/@ember/object/proxy.ts index d561d7777df..38dc2977b06 100644 --- a/packages/@ember/object/proxy.ts +++ b/packages/@ember/object/proxy.ts @@ -2,8 +2,20 @@ @module @ember/object/proxy */ +import { get, set } from '@ember/-internals/metal'; +import { meta } from '@ember/-internals/meta/lib/meta'; +import { tagForObject } from '@ember/-internals/metal/lib/tags'; +import { defineProperty } from '@ember/-internals/metal/lib/properties'; import { FrameworkObject } from '@ember/object/-internals'; -import _ProxyMixin from '@ember/-internals/runtime/lib/mixins/-proxy'; +import computed from '@ember/-internals/metal/lib/computed'; +import { setProxy } from '@ember/-internals/utils/lib/is_proxy'; +import { setCustomTagFor } from '@glimmer/manager/lib/util/args-proxy'; +import { + ContentProxy, + contentFor, + customTagForProxy, +} from '@ember/-internals/runtime/lib/proxy-utils'; +import { assert } from '@ember/debug'; /** `ObjectProxy` forwards all properties not defined by the proxy itself @@ -79,15 +91,20 @@ import _ProxyMixin from '@ember/-internals/runtime/lib/mixins/-proxy'; @class ObjectProxy @extends EmberObject - @uses Ember.ProxyMixin @public */ -interface ObjectProxy extends _ProxyMixin { - // Proxies forward to their content. This behavior *actually* comes from the - // ProxyMixin type itself, via its `unknownProperty` implementation, but if we - // try to apply it there, `ObjectProxy` does not correctly extend both the - // `EmberObject` and `ProxyMixin` types. Instead, we apply it here, and that - // gives us the desired behavior for this which actually *use* `ObjectProxy`. +interface ObjectProxy extends ContentProxy { + isTruthy: boolean; + + unknownProperty(key: K): Content[K] | undefined; + unknownProperty(key: string): unknown; + + setUnknownProperty(key: K, value: Content[K]): Content[K]; + setUnknownProperty(key: string, value: V): V; + + // Proxies forward to their content via `unknownProperty` and + // `setUnknownProperty`. The type overloads here merge Content and `this` + // so callers get the correct result type. get(keyName: K): Content[K]; get(keyname: K): this[K]; get(keyName: string): unknown; @@ -119,8 +136,49 @@ interface ObjectProxy extends _ProxyMixin { setProperties>(hash: T): T; } -// eslint-disable-next-line @typescript-eslint/no-unused-vars -class ObjectProxy extends FrameworkObject {} -ObjectProxy.PrototypeMixin.reopen(_ProxyMixin); +class ObjectProxy extends FrameworkObject { + content: unknown = null; + + init(...args: unknown[]) { + super.init(...args); + setProxy(this); + tagForObject(this); + setCustomTagFor(this, customTagForProxy); + } + + willDestroy() { + this.set('content', null); + super.willDestroy(); + } + + @computed('content') + get isTruthy(): boolean { + return Boolean(get(this, 'content')); + } + + unknownProperty(key: string): unknown { + let content = contentFor(this); + return content ? get(content, key) : undefined; + } + + setUnknownProperty(key: string, value: unknown): unknown { + let m = meta(this); + + if (m.isInitializing() || m.isPrototypeMeta(this)) { + defineProperty(this, key, null, value); + return value; + } + + let content = contentFor(this); + + assert( + `Cannot delegate set('${key}', ${value}) to the 'content' property of object proxy ${this}: its 'content' is undefined.`, + content + ); + + // SAFETY: We don't actually guarantee that this is an object, so this isn't necessarily safe :( + return set(content as object, key, value); + } +} export default ObjectProxy; diff --git a/packages/ember/tests/service_injection_test.js b/packages/ember/tests/service_injection_test.js index 1e6033f9e1d..5e105dfc54d 100644 --- a/packages/ember/tests/service_injection_test.js +++ b/packages/ember/tests/service_injection_test.js @@ -1,7 +1,6 @@ import { getOwner } from '@ember/-internals/owner'; import Controller from '@ember/controller'; import Service, { service } from '@ember/service'; -import { _ProxyMixin } from '@ember/-internals/runtime'; import { moduleFor, ApplicationTestCase } from 'internal-test-helpers'; import { computed } from '@ember/object'; import { precompileTemplate } from '@ember/template-compilation'; @@ -37,7 +36,7 @@ moduleFor( myService; } ); - let MyService = class extends Service.extend(_ProxyMixin) { + let MyService = class extends Service { init() { super.init(...arguments); diff --git a/tests/node-vitest/tree-shakability.test.js b/tests/node-vitest/tree-shakability.test.js index 815af53234c..7ffa1511c23 100644 --- a/tests/node-vitest/tree-shakability.test.js +++ b/tests/node-vitest/tree-shakability.test.js @@ -113,7 +113,6 @@ it('[dev] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/routing/index.js", "ember-source/@ember/-internals/runtime/index.js", "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", - "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", @@ -309,7 +308,6 @@ it('[prod] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/routing/index.js", "ember-source/@ember/-internals/runtime/index.js", "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", - "ember-source/@ember/-internals/runtime/lib/mixins/-proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", From de88732d370f88660e008f699bf652f544fd0999 Mon Sep 17 00:00:00 2001 From: Hubert Olender Date: Mon, 25 May 2026 16:47:47 +0200 Subject: [PATCH 6/6] refactor(mixin): remove registry and container proxy mixins - Inline RegistryProxyMixin behavior directly into Engine and EngineInstance - Inline ContainerProxyMixin behavior directly into EngineInstance - Remove RegistryProxyMixin and ContainerProxyMixin runtime exports - Delete the old runtime/lib/mixins/registry_proxy and container_proxy modules - Update package entrypoints and tree-shakability expectations - Move container proxy behavior coverage from the deleted mixin test to EngineInstance BREAKING CHANGE: private RegistryProxyMixin and ContainerProxyMixin mixin modules are removed. Engine and EngineInstance still provide the same registry/container proxy APIs directly. --- package.json | 2 - packages/@ember/-internals/owner/index.ts | 4 +- packages/@ember/-internals/runtime/index.ts | 2 - .../runtime/lib/mixins/container_proxy.ts | 59 ---------- .../runtime/lib/mixins/registry_proxy.ts | 62 ---------- .../tests/mixins/container_proxy_test.js | 70 ----------- packages/@ember/engine/index.ts | 50 +++++++- packages/@ember/engine/instance.ts | 110 +++++++++++++----- .../engine/tests/engine_instance_test.js | 55 ++++++++- tests/node-vitest/tree-shakability.test.js | 4 - 10 files changed, 181 insertions(+), 237 deletions(-) delete mode 100644 packages/@ember/-internals/runtime/lib/mixins/container_proxy.ts delete mode 100644 packages/@ember/-internals/runtime/lib/mixins/registry_proxy.ts delete mode 100644 packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js diff --git a/package.json b/package.json index ee8ff41c134..04bad3f0abf 100644 --- a/package.json +++ b/package.json @@ -188,8 +188,6 @@ "@ember/-internals/runtime/index.js": "ember-source/@ember/-internals/runtime/index.js", "@ember/-internals/runtime/lib/ext/rsvp.js": "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", "@ember/-internals/runtime/lib/mixins/action_handler.js": "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", - "@ember/-internals/runtime/lib/mixins/container_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", - "@ember/-internals/runtime/lib/mixins/registry_proxy.js": "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", "@ember/-internals/string/index.js": "ember-source/@ember/-internals/string/index.js", "@ember/-internals/utility-types/index.js": "ember-source/@ember/-internals/utility-types/index.js", "@ember/-internals/utils/index.js": "ember-source/@ember/-internals/utils/index.js", diff --git a/packages/@ember/-internals/owner/index.ts b/packages/@ember/-internals/owner/index.ts index d5d5e1123a1..14c2928c40b 100644 --- a/packages/@ember/-internals/owner/index.ts +++ b/packages/@ember/-internals/owner/index.ts @@ -560,12 +560,12 @@ export function setOwner(object: object, owner: Owner): void { glimmerSetOwner(object, owner); } -// Defines the type for the ContainerProxyMixin. When we rationalize our Owner +// Defines the container-facing portion of the Owner API. When we rationalize our Owner // *not* to work via mixins, we will be able to delete this entirely, in favor // of just using the Owner class itself. /** * The interface for a container proxy, which is itself a private API used - * by the private `ContainerProxyMixin` as part of the base definition of + * by concrete owner implementations as part of the base definition of * `EngineInstance`. * * @class ContainerProxy diff --git a/packages/@ember/-internals/runtime/index.ts b/packages/@ember/-internals/runtime/index.ts index b0228b1dc12..0fc85d90fe8 100644 --- a/packages/@ember/-internals/runtime/index.ts +++ b/packages/@ember/-internals/runtime/index.ts @@ -1,5 +1,3 @@ -export { default as RegistryProxyMixin } from './lib/mixins/registry_proxy'; -export { default as ContainerProxyMixin } from './lib/mixins/container_proxy'; export { default as ActionHandler } from './lib/mixins/action_handler'; export { contentFor as _contentFor } from './lib/proxy-utils'; diff --git a/packages/@ember/-internals/runtime/lib/mixins/container_proxy.ts b/packages/@ember/-internals/runtime/lib/mixins/container_proxy.ts deleted file mode 100644 index 9b9a870a7c9..00000000000 --- a/packages/@ember/-internals/runtime/lib/mixins/container_proxy.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { schedule, join } from '@ember/runloop'; -/** -@module ember -*/ -import type Container from '@ember/-internals/container/lib/container'; -import Mixin from '@ember/object/mixin'; -import type { ContainerProxy } from '@ember/-internals/owner'; - -// This is defined as a separate interface so that it can be used in the definition of -// `Owner` without also including the `__container__` property. - -/** - ContainerProxyMixin is used to provide public access to specific - container functionality. - - @class ContainerProxyMixin - @extends ContainerProxy - @private -*/ -interface ContainerProxyMixin extends ContainerProxy { - /** @internal */ - __container__: Container; -} -const ContainerProxyMixin = Mixin.create({ - /** - The container stores state. - - @private - @property {Ember.Container} __container__ - */ - __container__: null, - - ownerInjection() { - return this.__container__.ownerInjection(); - }, - - lookup(fullName: string, options: object) { - return this.__container__.lookup(fullName, options); - }, - - destroy() { - let container = this.__container__; - - if (container) { - join(() => { - container.destroy(); - schedule('destroy', container, 'finalizeDestroy'); - }); - } - - this._super(); - }, - - factoryFor(fullName: string) { - return this.__container__.factoryFor(fullName); - }, -}); - -export default ContainerProxyMixin; diff --git a/packages/@ember/-internals/runtime/lib/mixins/registry_proxy.ts b/packages/@ember/-internals/runtime/lib/mixins/registry_proxy.ts deleted file mode 100644 index 600b2bf62f2..00000000000 --- a/packages/@ember/-internals/runtime/lib/mixins/registry_proxy.ts +++ /dev/null @@ -1,62 +0,0 @@ -/** -@module ember -*/ - -import type Registry from '@ember/-internals/container/lib/registry'; -import type { RegistryProxy } from '@ember/-internals/owner'; -import type { AnyFn } from '@ember/-internals/utility-types'; - -import { assert } from '@ember/debug'; -import Mixin from '@ember/object/mixin'; - -/** - RegistryProxyMixin is used to provide public access to specific - registry functionality. - - @class RegistryProxyMixin - @extends RegistryProxy - @private -*/ -interface RegistryProxyMixin extends RegistryProxy { - /** @internal */ - __registry__: Registry; -} -const RegistryProxyMixin = Mixin.create({ - __registry__: null, - - resolveRegistration(fullName: string) { - assert('fullName must be a proper full name', this.__registry__.isValidFullName(fullName)); - return this.__registry__.resolve(fullName); - }, - - register: registryAlias('register'), - unregister: registryAlias('unregister'), - hasRegistration: registryAlias('has'), - registeredOption: registryAlias('getOption'), - registerOptions: registryAlias('options'), - registeredOptions: registryAlias('getOptions'), - registerOptionsForType: registryAlias('optionsForType'), - registeredOptionsForType: registryAlias('getOptionsForType'), -}); - -type AliasMethods = - | 'register' - | 'unregister' - | 'has' - | 'getOption' - | 'options' - | 'getOptions' - | 'optionsForType' - | 'getOptionsForType'; - -function registryAlias(name: N) { - return function (this: RegistryProxyMixin, ...args: Parameters) { - // We need this cast because `Parameters` is deferred so that it is not - // possible for TS to see it will always produce the right type. However, - // since `AnyFn` has a rest type, it is allowed. See discussion on [this - // issue](https://github.com/microsoft/TypeScript/issues/47615). - return (this.__registry__[name] as AnyFn)(...args); - }; -} - -export default RegistryProxyMixin; diff --git a/packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js b/packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js deleted file mode 100644 index 1f9cbcfa175..00000000000 --- a/packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js +++ /dev/null @@ -1,70 +0,0 @@ -import { getOwner } from '@ember/-internals/owner'; -import { Container, Registry } from '@ember/-internals/container'; -import ContainerProxy from '../../lib/mixins/container_proxy'; -import EmberObject from '@ember/object'; -import { run, schedule } from '@ember/runloop'; -import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; -import { destroy } from '@glimmer/destroyable'; - -moduleFor( - '@ember/-internals/runtime/mixins/container_proxy', - class extends AbstractTestCase { - beforeEach() { - this.Owner = EmberObject.extend(ContainerProxy); - this.instance = this.Owner.create(); - - this.registry = new Registry(); - - this.instance.__container__ = new Container(this.registry, { - owner: this.instance, - }); - } - - ['@test provides ownerInjection helper method'](assert) { - let result = this.instance.ownerInjection(); - - assert.equal(getOwner(result), this.instance, 'returns an object with an associated owner'); - } - - ['@test actions queue completes before destruction'](assert) { - assert.expect(1); - - this.registry.register( - 'service:auth', - class extends EmberObject { - willDestroy() { - assert.ok(getOwner(this).lookup('service:auth'), 'can still lookup'); - } - } - ); - - let service = this.instance.lookup('service:auth'); - - run(() => { - schedule('actions', service, 'destroy'); - this.instance.destroy(); - }); - } - - '@test being destroyed by @ember/destroyable properly destroys the container and created instances'( - assert - ) { - assert.expect(1); - - this.registry.register( - 'service:foo', - class FooService extends EmberObject { - willDestroy() { - assert.ok(true, 'is properly destroyed'); - } - } - ); - - this.instance.lookup('service:foo'); - - run(() => { - destroy(this.instance); - }); - } - } -); diff --git a/packages/@ember/engine/index.ts b/packages/@ember/engine/index.ts index 5a5df1fb583..0a26272e76f 100644 --- a/packages/@ember/engine/index.ts +++ b/packages/@ember/engine/index.ts @@ -14,7 +14,7 @@ import type { EngineInstanceOptions } from '@ember/engine/instance'; import EngineInstance from '@ember/engine/instance'; import { RoutingService } from '@ember/routing/-internals'; import { setupEngineRegistry } from '@ember/-internals/glimmer/lib/setup-registry'; -import RegistryProxyMixin from '@ember/-internals/runtime/lib/mixins/registry_proxy'; +import type { RegistryProxy, RegisterOptions, Factory, FullName } from '@ember/-internals/owner'; function props(obj: object) { let properties = []; @@ -50,12 +50,11 @@ export interface Initializer { @class Engine @extends Ember.Namespace - @uses RegistryProxyMixin @public */ // eslint-disable-next-line @typescript-eslint/no-empty-object-type -interface Engine extends RegistryProxyMixin {} -class Engine extends Namespace.extend(RegistryProxyMixin) { +interface Engine extends RegistryProxy {} +class Engine extends Namespace { static initializers: Record> = Object.create(null); static instanceInitializers: Record> = Object.create(null); @@ -329,6 +328,7 @@ class Engine extends Namespace.extend(RegistryProxyMixin) { @public */ declare Resolver: ResolverClass; + declare __registry__: Registry; init(properties: object | undefined) { super.init(properties); @@ -443,6 +443,48 @@ class Engine extends Namespace.extend(RegistryProxyMixin) { graph.topsort(cb); } + + // RegistryProxy implementation: + + resolveRegistration(fullName: FullName): Factory | object | undefined { + assert('fullName must be a proper full name', this.__registry__.isValidFullName(fullName)); + return this.__registry__.resolve(fullName); + } + + register(fullName: FullName, factory: Factory | object, options?: RegisterOptions) { + this.__registry__.register(fullName, factory, options); + } + + unregister(fullName: FullName) { + this.__registry__.unregister(fullName); + } + + hasRegistration(fullName: FullName): boolean { + return this.__registry__.has(fullName); + } + + registeredOption( + fullName: FullName, + optionName: K + ): RegisterOptions[K] | undefined { + return this.__registry__.getOption(fullName, optionName); + } + + registerOptions(fullName: FullName, options: RegisterOptions) { + this.__registry__.options(fullName, options); + } + + registeredOptions(fullName: FullName): RegisterOptions | undefined { + return this.__registry__.getOptions(fullName); + } + + registerOptionsForType(type: string, options: RegisterOptions) { + this.__registry__.optionsForType(type, options); + } + + registeredOptionsForType(type: string): RegisterOptions | undefined { + return this.__registry__.getOptionsForType(type); + } } /** diff --git a/packages/@ember/engine/instance.ts b/packages/@ember/engine/instance.ts index 050d27eb3b8..9e8a7aa4f89 100644 --- a/packages/@ember/engine/instance.ts +++ b/packages/@ember/engine/instance.ts @@ -7,16 +7,16 @@ import RSVP from '@ember/-internals/runtime/lib/ext/rsvp'; import { assert } from '@ember/debug'; import { default as Registry, privatize as P } from '@ember/-internals/container/lib/registry'; import { guidFor } from '@ember/-internals/utils/lib/guid'; +import { join, schedule } from '@ember/runloop'; import { ENGINE_PARENT, getEngineParent, setEngineParent } from './parent'; -import ContainerProxyMixin from '@ember/-internals/runtime/lib/mixins/container_proxy'; -import RegistryProxyMixin from '@ember/-internals/runtime/lib/mixins/registry_proxy'; -import type { InternalOwner } from '@ember/-internals/owner'; +import type { InternalOwner, RegisterOptions, Factory } from '@ember/-internals/owner'; import type Owner from '@ember/-internals/owner'; import { type FullName, isFactory } from '@ember/-internals/owner'; import type Engine from '@ember/engine'; import type Application from '@ember/application'; import type { BootEnvironment } from '@ember/-internals/glimmer/lib/views/outlet'; import type { SimpleElement } from '@simple-dom/interface'; +import type Container from '@ember/-internals/container/lib/container'; export interface BootOptions { isBrowser?: boolean; @@ -41,19 +41,11 @@ export interface EngineInstanceOptions { @public @class EngineInstance @extends EmberObject - @uses RegistryProxyMixin - @uses ContainerProxyMixin */ -// Note on types: since `EngineInstance` uses `RegistryProxyMixin` and -// `ContainerProxyMixin`, which respectively implement the same `RegistryMixin` -// and `ContainerMixin` types used to define `InternalOwner`, this is the same -// type as `InternalOwner` from TS's POV. The point of the explicit `extends` -// clauses for `InternalOwner` and `Owner` is to keep us honest: if this stops -// type checking, we have broken part of our public API contract. Medium-term, -// the goal here is to `EngineInstance` simple be `Owner`. -interface EngineInstance extends RegistryProxyMixin, ContainerProxyMixin, InternalOwner, Owner {} -class EngineInstance extends EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin) { +// `EngineInstance` implements the registry/container proxy APIs directly. +interface EngineInstance extends InternalOwner, Owner {} +class EngineInstance extends EmberObject { /** @private @method setupRegistry @@ -82,6 +74,9 @@ class EngineInstance extends EmberObject.extend(RegistryProxyMixin, ContainerPro _booted = false; + declare __container__: Container; + declare __registry__: Registry; + init(properties: object | undefined) { super.init(properties); @@ -170,23 +165,6 @@ class EngineInstance extends EmberObject.extend(RegistryProxyMixin, ContainerPro (this.constructor as typeof EngineInstance).setupRegistry(this.__registry__, options); } - /** - Unregister a factory. - - Overrides `RegistryProxy#unregister` in order to clear any cached instances - of the unregistered factory. - - @public - @method unregister - @param {String} fullName - */ - unregister(fullName: FullName) { - this.__container__.reset(fullName); - - // We overwrote this method from RegistryProxyMixin. - this.__registry__.unregister(fullName); - } - /** Build a new `EngineInstance` that's a child of this instance. @@ -257,6 +235,76 @@ class EngineInstance extends EmberObject.extend(RegistryProxyMixin, ContainerPro this.register(key, singleton, { instantiate: false }); }); } + + // ContainerProxy implementation: + + ownerInjection() { + return this.__container__.ownerInjection(); + } + + lookup(fullName: FullName, options?: RegisterOptions) { + return this.__container__.lookup(fullName, options); + } + + factoryFor(fullName: FullName) { + return this.__container__.factoryFor(fullName); + } + + destroy() { + let container = this.__container__; + + if (container) { + join(() => { + container.destroy(); + schedule('destroy', container, 'finalizeDestroy'); + }); + } + + super.destroy(); + } + + // RegistryProxy implementation: + + resolveRegistration(fullName: FullName): Factory | object | undefined { + assert('fullName must be a proper full name', this.__registry__.isValidFullName(fullName)); + return this.__registry__.resolve(fullName); + } + + register(fullName: FullName, factory: Factory | object, options?: RegisterOptions) { + this.__registry__.register(fullName, factory, options); + } + + unregister(fullName: FullName) { + this.__container__.reset(fullName); + this.__registry__.unregister(fullName); + } + + hasRegistration(fullName: FullName): boolean { + return this.__registry__.has(fullName); + } + + registeredOption( + fullName: FullName, + optionName: K + ): RegisterOptions[K] | undefined { + return this.__registry__.getOption(fullName, optionName); + } + + registerOptions(fullName: FullName, options: RegisterOptions) { + this.__registry__.options(fullName, options); + } + + registeredOptions(fullName: FullName): RegisterOptions | undefined { + return this.__registry__.getOptions(fullName); + } + + registerOptionsForType(type: string, options: RegisterOptions) { + this.__registry__.optionsForType(type, options); + } + + registeredOptionsForType(type: string): RegisterOptions | undefined { + return this.__registry__.getOptionsForType(type); + } } export default EngineInstance; diff --git a/packages/@ember/engine/tests/engine_instance_test.js b/packages/@ember/engine/tests/engine_instance_test.js index fe5f0c2a1b0..00034da5cf1 100644 --- a/packages/@ember/engine/tests/engine_instance_test.js +++ b/packages/@ember/engine/tests/engine_instance_test.js @@ -1,6 +1,9 @@ import Engine, { getEngineParent, setEngineParent } from '@ember/engine'; import EngineInstance from '@ember/engine/instance'; -import { run } from '@ember/runloop'; +import EmberObject from '@ember/object'; +import { run, schedule } from '@ember/runloop'; +import { getOwner } from '@ember/-internals/owner'; +import { destroy } from '@glimmer/destroyable'; import { factory } from 'internal-test-helpers'; import { moduleFor, @@ -70,6 +73,56 @@ moduleFor( ); } + ['@test provides ownerInjection helper method'](assert) { + engineInstance = run(() => EngineInstance.create({ base: engine })); + + let result = engineInstance.ownerInjection(); + + assert.equal(getOwner(result), engineInstance, 'returns an object with an associated owner'); + } + + ['@test actions queue completes before destruction'](assert) { + assert.expect(1); + + engineInstance = run(() => EngineInstance.create({ base: engine })); + + let AuthService = class extends EmberObject { + willDestroy() { + assert.ok(getOwner(this).lookup('service:auth'), 'can still lookup'); + } + }; + + engineInstance.register('service:auth', AuthService); + + let service = engineInstance.lookup('service:auth'); + + run(() => { + schedule('actions', service, 'destroy'); + engineInstance.destroy(); + }); + } + + ['@test being destroyed by @ember/destroyable properly destroys the container and created instances']( + assert + ) { + assert.expect(1); + + engineInstance = run(() => EngineInstance.create({ base: engine })); + + let FooService = class extends EmberObject { + willDestroy() { + assert.ok(true, 'is properly destroyed'); + } + }; + + engineInstance.register('service:foo', FooService); + engineInstance.lookup('service:foo'); + + run(() => { + destroy(engineInstance); + }); + } + ['@test can be booted when its parent has been set'](assert) { assert.expect(3); diff --git a/tests/node-vitest/tree-shakability.test.js b/tests/node-vitest/tree-shakability.test.js index 7ffa1511c23..d543e4b3524 100644 --- a/tests/node-vitest/tree-shakability.test.js +++ b/tests/node-vitest/tree-shakability.test.js @@ -114,8 +114,6 @@ it('[dev] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/runtime/index.js", "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", - "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", - "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", "ember-source/@ember/-internals/utils/index.js", "ember-source/@ember/-internals/views/index.js", "ember-source/@ember/-internals/views/lib/compat/fallback-view-registry.js", @@ -309,8 +307,6 @@ it('[prod] has expected tree-shakable entrypoints', async () => { "ember-source/@ember/-internals/runtime/index.js", "ember-source/@ember/-internals/runtime/lib/ext/rsvp.js", "ember-source/@ember/-internals/runtime/lib/mixins/action_handler.js", - "ember-source/@ember/-internals/runtime/lib/mixins/container_proxy.js", - "ember-source/@ember/-internals/runtime/lib/mixins/registry_proxy.js", "ember-source/@ember/-internals/utils/index.js", "ember-source/@ember/-internals/views/index.js", "ember-source/@ember/-internals/views/lib/compat/fallback-view-registry.js",