Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: lookup/register and singleton flag behavior #19491

Merged
merged 2 commits into from
May 24, 2021
Merged

fix: lookup/register and singleton flag behavior #19491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d49ea9
fix: lookup/register and singleton flag behavior
runspired Apr 6, 2021
6706b17
fix lint
runspired Apr 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/@ember/-internals/container/lib/container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,10 @@ function isInstantiatable(container: Container, fullName: string) {
function lookup(container: Container, fullName: string, options: LookupOptions = {}) {
let normalizedName = fullName;

if (options.singleton !== false) {
if (
options.singleton === true ||
(options.singleton === undefined && isSingleton(container, fullName))
) {
let cached = container.cache[normalizedName];
if (cached !== undefined) {
return cached;
Expand Down Expand Up @@ -335,7 +338,7 @@ function isSingletonInstance(
return (
singleton !== false &&
instantiate !== false &&
isSingleton(container, fullName) &&
(singleton === true || isSingleton(container, fullName)) &&
isInstantiatable(container, fullName)
);
}
Expand All @@ -359,7 +362,7 @@ function isFactoryInstance(
) {
return (
instantiate !== false &&
(singleton !== false || isSingleton(container, fullName)) &&
(singleton === false || !isSingleton(container, fullName)) &&
isInstantiatable(container, fullName)
);
}
Expand Down
224 changes: 183 additions & 41 deletions packages/@ember/-internals/container/tests/container_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,48 +6,9 @@ import { Registry } from '..';
import { factory, moduleFor, AbstractTestCase, runTask } from 'internal-test-helpers';

moduleFor(
'Container',
'Container.lookup',
class extends AbstractTestCase {
['@test A registered factory returns the same instance each time'](assert) {
let registry = new Registry();
let container = registry.container();
let PostController = factory();

registry.register('controller:post', PostController);

let postController = container.lookup('controller:post');

assert.ok(
postController instanceof PostController,
'The lookup is an instance of the factory'
);

assert.equal(postController, container.lookup('controller:post'));
}

['@test uses create time injections if factory has no extend'](assert) {
let registry = new Registry();
let container = registry.container();
let AppleController = factory();
let PostController = factory();

PostController.extend = undefined; // remove extend

registry.register('controller:apple', AppleController);
registry.register('controller:post', PostController);
registry.injection('controller:post', 'apple', 'controller:apple');

let postController = container.lookup('controller:post');

assert.ok(
postController.apple instanceof AppleController,
'instance receives an apple of instance AppleController'
);
}

['@test A registered factory returns a fresh instance if singleton: false is passed as an option'](
assert
) {
['@test lookup returns a fresh instance if singleton: false is passed as an option'](assert) {
let registry = new Registry();
let container = registry.container();
let PostController = factory();
Expand Down Expand Up @@ -102,6 +63,187 @@ moduleFor(
);
}

['@test lookup returns a fresh instance if singleton: false is passed as an option to lookup'](
assert
) {
class TestFactory {
constructor(opts) {
Object.assign(this, opts);
}
static create(opts) {
return new this(opts);
}
}

let registry = new Registry();
let container = registry.container();
registry.register('thing:test/obj', TestFactory);

let instance1 = container.lookup('thing:test/obj');
let instance2 = container.lookup('thing:test/obj', {
singleton: false,
});
let instance3 = container.lookup('thing:test/obj', {
singleton: false,
});
let instance4 = container.lookup('thing:test/obj');

assert.ok(
instance1 === instance4,
'factories looked up up without singleton: false are the same instance'
);
assert.ok(
instance1 !== instance2,
'factories looked up with singleton: false are a different instance'
);
assert.ok(
instance2 !== instance3,
'factories looked up with singleton: false are a different instance'
);
assert.ok(
instance3 !== instance4,
'factories looked up after a call to singleton: false is a different instance'
);
assert.ok(
instance1 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance2 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance3 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance4 instanceof TestFactory,
'All instances are instances of the registered factory'
);
}

['@test lookup returns a fresh instance if singleton: false is passed as an option to register'](
assert
) {
class TestFactory {
constructor(opts) {
Object.assign(this, opts);
}
static create(opts) {
return new this(opts);
}
}

let registry = new Registry();
let container = registry.container();
registry.register('thing:test/obj', TestFactory, { singleton: false });

let instance1 = container.lookup('thing:test/obj');
let instance2 = container.lookup('thing:test/obj');
let instance3 = container.lookup('thing:test/obj');

assert.ok(instance1 !== instance2, 'each lookup is a different instance');
assert.ok(instance2 !== instance3, 'each lookup is a different instance');
assert.ok(instance1 !== instance3, 'each lookup is a different instance');
assert.ok(
instance1 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance2 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance3 instanceof TestFactory,
'All instances are instances of the registered factory'
);
}

['@test lookup returns a singleton instance if singleton: true is passed as an option even if registered as singleton: false'](
assert
) {
class TestFactory {
constructor(opts) {
Object.assign(this, opts);
}
static create(opts) {
return new this(opts);
}
}

let registry = new Registry();
let container = registry.container();
registry.register('thing:test/obj', TestFactory, { singleton: false });

let instance1 = container.lookup('thing:test/obj');
let instance2 = container.lookup('thing:test/obj', { singleton: true });
let instance3 = container.lookup('thing:test/obj', { singleton: true });
let instance4 = container.lookup('thing:test/obj');

assert.ok(instance1 !== instance2, 'each lookup is a different instance');
assert.ok(instance2 === instance3, 'each singleton: true lookup is the same instance');
assert.ok(instance3 !== instance4, 'each lookup is a different instance');
assert.ok(instance1 !== instance4, 'each lookup is a different instance');
assert.ok(
instance1 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance2 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance3 instanceof TestFactory,
'All instances are instances of the registered factory'
);
assert.ok(
instance4 instanceof TestFactory,
'All instances are instances of the registered factory'
);
}
}
);

moduleFor(
'Container',
class extends AbstractTestCase {
['@test A registered factory returns the same instance each time'](assert) {
let registry = new Registry();
let container = registry.container();
let PostController = factory();

registry.register('controller:post', PostController);

let postController = container.lookup('controller:post');

assert.ok(
postController instanceof PostController,
'The lookup is an instance of the factory'
);

assert.equal(postController, container.lookup('controller:post'));
}

['@test uses create time injections if factory has no extend'](assert) {
let registry = new Registry();
let container = registry.container();
let AppleController = factory();
let PostController = factory();

PostController.extend = undefined; // remove extend

registry.register('controller:apple', AppleController);
registry.register('controller:post', PostController);
registry.injection('controller:post', 'apple', 'controller:apple');

let postController = container.lookup('controller:post');

assert.ok(
postController.apple instanceof AppleController,
'instance receives an apple of instance AppleController'
);
}

["@test A factory type with a registered injection's instances receive that injection"](
assert
) {
Expand Down