diff --git a/packages/@ember/-internals/runtime/lib/system/core_object.js b/packages/@ember/-internals/runtime/lib/system/core_object.js index a8f209d9153..f90d38fdd3b 100644 --- a/packages/@ember/-internals/runtime/lib/system/core_object.js +++ b/packages/@ember/-internals/runtime/lib/system/core_object.js @@ -39,6 +39,14 @@ const prototypeMixinMap = new WeakMap(); const initCalled = DEBUG ? new WeakSet() : undefined; // only used in debug builds to enable the proxy trap +const destroyCalled = new Set(); + +function ensureDestroyCalled(instance) { + if (!destroyCalled.has(instance)) { + instance.destroy(); + } +} + function initialize(obj, properties) { let m = meta(obj); @@ -256,6 +264,7 @@ class CoreObject { }); } + registerDestructor(self, ensureDestroyCalled, true); registerDestructor(self, () => self.willDestroy()); // disable chains @@ -512,7 +521,15 @@ class CoreObject { @public */ destroy() { - destroy(this); + // Used to ensure that manually calling `.destroy()` does not immediately call destroy again + destroyCalled.add(this); + + try { + destroy(this); + } finally { + destroyCalled.delete(this); + } + return this; } diff --git a/packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js b/packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js index 860be7f0005..7119e9616e3 100644 --- a/packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js +++ b/packages/@ember/-internals/runtime/tests/mixins/container_proxy_test.js @@ -4,6 +4,7 @@ import ContainerProxy from '../../lib/mixins/container_proxy'; import EmberObject from '../../lib/system/object'; import { run, schedule } from '@ember/runloop'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; +import { destroy } from '@glimmer/runtime'; moduleFor( '@ember/-internals/runtime/mixins/container_proxy', @@ -44,5 +45,26 @@ moduleFor( 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/-internals/runtime/tests/system/core_object_test.js b/packages/@ember/-internals/runtime/tests/system/core_object_test.js index 7007285dd5a..c788ad15e46 100644 --- a/packages/@ember/-internals/runtime/tests/system/core_object_test.js +++ b/packages/@ember/-internals/runtime/tests/system/core_object_test.js @@ -3,6 +3,8 @@ import { get, set, observer } from '@ember/-internals/metal'; import CoreObject from '../../lib/system/core_object'; import { moduleFor, AbstractTestCase, buildOwner, runLoopSettled } from 'internal-test-helpers'; import { track } from '@glimmer/validator'; +import { destroy } from '@glimmer/runtime'; +import { run } from '@ember/runloop'; moduleFor( 'Ember.CoreObject', @@ -129,5 +131,20 @@ moduleFor( assert.ok(true, 'We did not error'); }); } + '@test destroy method is called when being destroyed by @ember/destroyable'(assert) { + assert.expect(1); + + class TestObject extends CoreObject { + destroy() { + assert.ok(true, 'destroy was invoked'); + } + } + + let instance = TestObject.create(); + + run(() => { + destroy(instance); + }); + } } ); diff --git a/packages/@ember/application/tests/reset_test.js b/packages/@ember/application/tests/reset_test.js index 1cbb81e9899..b3454826dc7 100644 --- a/packages/@ember/application/tests/reset_test.js +++ b/packages/@ember/application/tests/reset_test.js @@ -3,6 +3,7 @@ import { get } from '@ember/-internals/metal'; import Controller from '@ember/controller'; import { Router } from '@ember/-internals/routing'; import { moduleFor, AutobootApplicationTestCase } from 'internal-test-helpers'; +import { CoreObject } from '@ember/-internals/runtime'; moduleFor( 'Application - resetting', @@ -71,31 +72,35 @@ moduleFor( let eventDispatcherWasSetup = 0; let eventDispatcherWasDestroyed = 0; - let mockEventDispatcher = { + class FakeEventDispatcher extends CoreObject { setup() { eventDispatcherWasSetup++; - }, + } + destroy() { + if (this.isDestroying) { + return; + } + super.destroy(); + eventDispatcherWasDestroyed++; - }, - }; + } + } run(() => { this.createApplication(); - this.add('event_dispatcher:main', { - create: () => mockEventDispatcher, - }); + this.add('event_dispatcher:main', FakeEventDispatcher); - assert.equal(eventDispatcherWasSetup, 0); - assert.equal(eventDispatcherWasDestroyed, 0); + assert.equal(eventDispatcherWasSetup, 0, 'not setup yet'); + assert.equal(eventDispatcherWasDestroyed, 0, 'not destroyed yet'); }); - assert.equal(eventDispatcherWasSetup, 1); - assert.equal(eventDispatcherWasDestroyed, 0); + assert.equal(eventDispatcherWasSetup, 1, 'setup'); + assert.equal(eventDispatcherWasDestroyed, 0, 'still not destroyed'); this.application.reset(); - assert.equal(eventDispatcherWasDestroyed, 1); + assert.equal(eventDispatcherWasDestroyed, 1, 'destroyed'); assert.equal(eventDispatcherWasSetup, 2, 'setup called after reset'); }