Skip to content

Commit

Permalink
[BUGFIX lts] Revert container deprecation
Browse files Browse the repository at this point in the history
We agreed that it was important to fix the error cases in #18717, but
while the work may be unnecessary, there isn't necessarily any
"correctness" issues with looking up things during destruction. Using
a deprecation to warn about potentially "unnecessary" work is a bit
heavy-handed, and at minimum requires some more continued discussions,
so reverting that part of the PR for now (while keeping the errors in
tact).

(cherry picked from commit b853324)
  • Loading branch information
chancancode committed Feb 8, 2020
1 parent 4f10138 commit 055fab0
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 78 deletions.
10 changes: 1 addition & 9 deletions packages/@ember/-internals/container/lib/container.ts
Original file line number Diff line number Diff line change
@@ -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, deprecate } from '@ember/debug';
import { assert } from '@ember/debug';
import { assign } from '@ember/polyfills';
import { DEBUG } from '@glimmer/env';
import Registry, { DebugRegistry, Injection } from './registry';
Expand Down Expand Up @@ -584,14 +584,6 @@ class FactoryManager<T, C> {
);
}

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);
Expand Down
69 changes: 0 additions & 69 deletions packages/@ember/-internals/container/tests/container_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -736,75 +736,6 @@ moduleFor(
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
) {
Expand Down

0 comments on commit 055fab0

Please sign in to comment.