diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 5b8cb1c23ef..23996f7b198 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -1,7 +1,7 @@ import { Factory, LookupOptions, Owner, OWNER, setOwner } from '@ember/-internals/owner'; import { dictionary, HAS_NATIVE_PROXY } from '@ember/-internals/utils'; import { EMBER_MODULE_UNIFICATION } from '@ember/canary-features'; -import { assert } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { assign } from '@ember/polyfills'; import { DEBUG } from '@glimmer/env'; import Registry, { DebugRegistry, Injection } from './registry'; @@ -149,7 +149,9 @@ export default class Container { @return {any} */ lookup(fullName: string, options: LookupOptions): any { - assert('expected container not to be destroyed', !this.isDestroyed); + if (this.isDestroyed) { + throw new Error(`Can not call \`.lookup\` after the owner has been destroyed`); + } assert('fullName must be a proper full name', this.registry.isValidFullName(fullName)); return lookup(this, this.registry.normalize(fullName), options); } @@ -161,8 +163,9 @@ export default class Container { @method destroy */ destroy(): void { - destroyDestroyables(this); this.isDestroying = true; + + destroyDestroyables(this); } finalizeDestroy(): void { @@ -211,7 +214,9 @@ export default class Container { @return {any} */ factoryFor(fullName: string, options: LookupOptions = {}): Factory | undefined { - assert('expected container not to be destroyed', !this.isDestroyed); + if (this.isDestroyed) { + throw new Error(`Can not call \`.factoryFor\` after the owner has been destroyed`); + } let normalizedName = this.registry.normalize(fullName); assert('fullName must be a proper full name', this.registry.isValidFullName(normalizedName)); @@ -397,7 +402,17 @@ function instantiateFactory( // SomeClass { singleton: true, instantiate: true } | { singleton: true } | { instantiate: true } | {} // By default majority of objects fall into this case if (isSingletonInstance(container, fullName, options)) { - return (container.cache[normalizedName] = factoryManager.create()); + let instance = (container.cache[normalizedName] = factoryManager.create()); + + // if this lookup happened _during_ destruction (emits a deprecation, but + // is still possible) ensure that it gets destroyed + if (container.isDestroying) { + if (typeof instance.destroy === 'function') { + instance.destroy(); + } + } + + return instance; } // SomeClass { singleton: false, instantiate: true } @@ -561,6 +576,22 @@ class FactoryManager { } create(options?: { [prop: string]: any }) { + let { container } = this; + + if (container.isDestroyed) { + throw new Error( + `Can not create new instances after the owner has been destroyed (you attempted to create ${this.fullName})` + ); + } + + if (DEBUG) { + deprecate( + `Instantiating a new instance of ${this.fullName} while the owner is being destroyed is deprecated.`, + !container.isDestroying, + { id: 'container.lookup-on-destroy', until: '3.20.0' } + ); + } + let injectionsCache = this.injections; if (injectionsCache === undefined) { let { injections, isDynamic } = injectionsFor(this.container, this.normalizedName); diff --git a/packages/@ember/-internals/container/tests/container_test.js b/packages/@ember/-internals/container/tests/container_test.js index 2a5bdf5e944..f0b5582b1a6 100644 --- a/packages/@ember/-internals/container/tests/container_test.js +++ b/packages/@ember/-internals/container/tests/container_test.js @@ -680,9 +680,9 @@ moduleFor( [`@test assert when calling lookup after destroy on a container`](assert) { let registry = new Registry(); let container = registry.container(); - let Component = factory(); - registry.register('component:foo-bar', Component); - let instance = container.lookup('component:foo-bar'); + registry.register('service:foo', factory()); + + let instance = container.lookup('service:foo'); assert.ok(instance, 'precond lookup successful'); runTask(() => { @@ -690,17 +690,17 @@ moduleFor( container.finalizeDestroy(); }); - expectAssertion(() => { - container.lookup('component:foo-bar'); - }); + assert.throws(() => { + container.lookup('service:foo'); + }, /Can not call `.lookup` after the owner has been destroyed/); } [`@test assert when calling factoryFor after destroy on a container`](assert) { let registry = new Registry(); let container = registry.container(); - let Component = factory(); - registry.register('component:foo-bar', Component); - let instance = container.factoryFor('component:foo-bar'); + registry.register('service:foo', factory()); + + let instance = container.lookup('service:foo'); assert.ok(instance, 'precond lookup successful'); runTask(() => { @@ -708,9 +708,9 @@ moduleFor( container.finalizeDestroy(); }); - expectAssertion(() => { - container.factoryFor('component:foo-bar'); - }); + assert.throws(() => { + container.factoryFor('service:foo'); + }, /Can not call `.factoryFor` after the owner has been destroyed/); } // this is skipped until templates and the glimmer environment do not require `OWNER` to be @@ -735,6 +735,94 @@ moduleFor( // not via registry/container shenanigans assert.deepEqual(Object.keys(instance), []); } + + '@test instantiating via container.lookup during destruction emits a deprecation'(assert) { + let registry = new Registry(); + let container = registry.container(); + class Service extends factory() { + destroy() { + expectDeprecation(() => { + container.lookup('service:other'); + }, /Instantiating a new instance of service:other while the owner is being destroyed is deprecated/); + } + } + registry.register('service:foo', Service); + registry.register('service:other', factory()); + let instance = container.lookup('service:foo'); + assert.ok(instance, 'precond lookup successful'); + + runTask(() => { + container.destroy(); + container.finalizeDestroy(); + }); + } + + '@test instantiating via container.lookup during destruction enqueues destruction'(assert) { + let registry = new Registry(); + let container = registry.container(); + let otherInstance; + class Service extends factory() { + destroy() { + expectDeprecation(() => { + otherInstance = container.lookup('service:other'); + }, /Instantiating a new instance of service:other while the owner is being destroyed is deprecated/); + + assert.ok(otherInstance.isDestroyed, 'service:other was destroyed'); + } + } + registry.register('service:foo', Service); + registry.register('service:other', factory()); + let instance = container.lookup('service:foo'); + assert.ok(instance, 'precond lookup successful'); + + runTask(() => { + container.destroy(); + container.finalizeDestroy(); + }); + } + + '@test instantiating via container.factoryFor().create() during destruction emits a deprecation'( + assert + ) { + let registry = new Registry(); + let container = registry.container(); + class Service extends factory() { + destroy() { + expectDeprecation(() => { + let Factory = container.factoryFor('service:other'); + Factory.create(); + }, /Instantiating a new instance of service:other while the owner is being destroyed is deprecated/); + } + } + registry.register('service:foo', Service); + registry.register('service:other', factory()); + let instance = container.lookup('service:foo'); + assert.ok(instance, 'precond lookup successful'); + + runTask(() => { + container.destroy(); + container.finalizeDestroy(); + }); + } + + '@test instantiating via container.factoryFor().create() after destruction throws an error'( + assert + ) { + let registry = new Registry(); + let container = registry.container(); + registry.register('service:foo', factory()); + registry.register('service:other', factory()); + let Factory = container.factoryFor('service:other'); + + runTask(() => { + container.destroy(); + container.finalizeDestroy(); + }); + + assert.throws(() => { + Factory.create(); + }, /Can not create new instances after the owner has been destroyed \(you attempted to create service:other\)/); + } } );