diff --git a/packages/container/lib/container.js b/packages/container/lib/container.js index 8466ee614dd..e1980052360 100644 --- a/packages/container/lib/container.js +++ b/packages/container/lib/container.js @@ -142,9 +142,11 @@ Container.prototype = { lookupFactory(fullName, options) { assert('fullName must be a proper full name', this.registry.validateFullName(fullName)); - if (isFeatureEnabled('ember-no-double-extend')) { - deprecate('Using "_lookupFactory" is deprecated. Please use container.factoryFor instead.', false, { id: 'container-lookupFactory', until: '2.13.0', url: 'TODO' }); - } + deprecate( + 'Using "_lookupFactory" is deprecated. Please use container.factoryFor instead.', + !isFeatureEnabled('ember-factory-for'), + { id: 'container-lookupFactory', until: '2.13.0', url: 'TODO' } + ); return deprecatedFactoryFor(this, this.registry.normalize(fullName), options); }, @@ -154,57 +156,27 @@ Container.prototype = { return deprecatedFactoryFor(this, this.registry.normalize(fullName), options); }, + /* + * This internal version of factoryFor swaps between the public API for + * factoryFor (class is the registered class) and a transition implementation + * (class is the double-extended class). It is *not* the public API version + * of factoryFor, which always returns the registered class. + */ [FACTORY_FOR](fullName, options = {}) { - let manager; if (isFeatureEnabled('ember-no-double-extend')) { - let normalizedName = this.registry.normalize(fullName); - assert('fullName must be a proper full name', this.registry.validateFullName(normalizedName)); - - if (options.source) { - normalizedName = this.registry.expandLocalLookup(fullName, options); - // if expandLocalLookup returns falsey, we do not support local lookup - if (!normalizedName) { return; } + if (isFeatureEnabled('ember-factory-for')) { + return this.factoryFor(fullName, options); + } else { + /* This throws in case of a poorly designed build */ + throw new Error('If ember-no-double-extend is enabled, ember-factory-for must also be enabled'); } - - let factory = this.registry.resolve(normalizedName); - - if (factory === undefined) { return; } - - manager = new FactoryManager(this, factory, fullName, normalizedName); - } else { - let factory = this.lookupFactory(fullName, options); - if (factory === undefined) { return; } - manager = new DeprecatedFactoryManager(this, factory, fullName); } + let factory = this.lookupFactory(fullName, options); + if (factory === undefined) { return; } + let manager = new DeprecatedFactoryManager(this, factory, fullName); runInDebug(() => { - if (HAS_PROXY) { - let validator = { - get(obj, prop) { - if (prop !== 'class' && prop !== 'create') { - throw new Error(`You attempted to access "${prop}" on a factory manager created by container#factoryFor. "${prop}" is not a member of a factory manager."`); - } - - return obj[prop]; - }, - set(obj, prop, value) { - throw new Error(`You attempted to set "${prop}" on a factory manager created by container#factoryFor. A factory manager is a read-only construct.`); - } - }; - - // Note: - // We have to proxy access to the manager here so that private property - // access doesn't cause the above errors to occur. - let m = manager; - let proxiedManager = { - class: m.class, - create(props) { - return m.create(props); - } - }; - - manager = new Proxy(proxiedManager, validator); - } + manager = wrapManagerInDeprecationProxy(manager); }); return manager; @@ -255,10 +227,48 @@ Container.prototype = { } }; +/* + * Wrap a factory manager in a proxy which will not permit properties to be + * set on the manager. + */ +function wrapManagerInDeprecationProxy(manager) { + if (HAS_PROXY) { + let validator = { + get(obj, prop) { + if (prop !== 'class' && prop !== 'create') { + throw new Error(`You attempted to access "${prop}" on a factory manager created by container#factoryFor. "${prop}" is not a member of a factory manager."`); + } + + return obj[prop]; + }, + set(obj, prop, value) { + throw new Error(`You attempted to set "${prop}" on a factory manager created by container#factoryFor. A factory manager is a read-only construct.`); + } + }; + + // Note: + // We have to proxy access to the manager here so that private property + // access doesn't cause the above errors to occur. + let m = manager; + let proxiedManager = { + class: m.class, + create(props) { + return m.create(props); + } + }; + + return new Proxy(proxiedManager, validator); + } + + return manager; +} + if (isFeatureEnabled('ember-factory-for')) { /** - Given a fullName, return the corresponding factory. The consumer of factory is repsonsible - for the destruction of the factory as there is no way to understand the objects lifecycle. + Given a fullName, return the corresponding factory. The consumer of the factory + is responsible for the destruction of any factory instances, as there is no + way for the container to ensure instances are destroyed when it itself is + destroyed. @public @method factoryFor @@ -267,8 +277,27 @@ if (isFeatureEnabled('ember-factory-for')) { @param {String} [options.source] The fullname of the request source (used for local lookup) @return {any} */ - Container.prototype.factoryFor = function _factoryFor() { - return this[FACTORY_FOR](...arguments); + Container.prototype.factoryFor = function _factoryFor(fullName, options = {}) { + let normalizedName = this.registry.normalize(fullName); + assert('fullName must be a proper full name', this.registry.validateFullName(normalizedName)); + + if (options.source) { + normalizedName = this.registry.expandLocalLookup(fullName, options); + // if expandLocalLookup returns falsey, we do not support local lookup + if (!normalizedName) { return; } + } + + let factory = this.registry.resolve(normalizedName); + + if (factory === undefined) { return; } + + let manager = new FactoryManager(this, factory, fullName, normalizedName); + + runInDebug(() => { + manager = wrapManagerInDeprecationProxy(manager); + }); + + return manager; }; } diff --git a/packages/container/tests/container_test.js b/packages/container/tests/container_test.js index e4a54f40cbf..bfa94db34b1 100644 --- a/packages/container/tests/container_test.js +++ b/packages/container/tests/container_test.js @@ -3,6 +3,7 @@ import { ENV } from 'ember-environment'; import { get, isFeatureEnabled } from 'ember-metal'; import { Registry } from '../index'; import { factory } from 'internal-test-helpers'; +import { FACTORY_FOR } from 'container'; let originalModelInjections; @@ -18,7 +19,7 @@ QUnit.module('Container', { function lookupFactory(name, container, options) { let factory; if (isFeatureEnabled('ember-no-double-extend')) { - expectDeprecation(() => { + ignoreDeprecation(() => { factory = container.lookupFactory(name, options); }); } else { @@ -489,11 +490,7 @@ QUnit.test('factory for non extendables resolves are cached', function() { }); QUnit.test('The `_onLookup` hook is called on factories when looked up the first time', function() { - if (isFeatureEnabled('ember-factory-for') && isFeatureEnabled('ember-no-double-extend')) { - expect(4); - } else { - expect(2); - } + expect(2); let registry = new Registry(); let container = registry.container(); @@ -701,6 +698,20 @@ QUnit.test('lookup passes options through to expandlocallookup', function(assert assert.ok(PostControllerLookupResult instanceof PostController); }); +QUnit.test('#[FACTORY_FOR] class is the injected factory', (assert) => { + let registry = new Registry(); + let container = registry.container(); + + let Component = factory(); + registry.register('component:foo-bar', Component); + + let factoryCreator = container[FACTORY_FOR]('component:foo-bar'); + if (isFeatureEnabled('ember-no-double-extend')) { + assert.deepEqual(factoryCreator.class, Component, 'No double extend'); + } else { + assert.deepEqual(factoryCreator.class, container.lookupFactory('component:foo-bar'), 'Double extended class'); + } +}); if (isFeatureEnabled('ember-factory-for')) { QUnit.test('#factoryFor must supply a fullname', (assert) => { @@ -731,11 +742,7 @@ if (isFeatureEnabled('ember-factory-for')) { registry.register('component:foo-bar', Component); let factoryCreator = container.factoryFor('component:foo-bar'); - if (isFeatureEnabled('ember-no-double-extend')) { - assert.deepEqual(factoryCreator.class, Component, 'No double extend'); - } else { - assert.notDeepEqual(factoryCreator.class, Component, 'Double extended class'); - } + assert.deepEqual(factoryCreator.class, Component, 'No double extend'); }); QUnit.test('#factoryFor instance have a common parent', (assert) => { diff --git a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js index 9fc4f33418e..07d1daac02f 100644 --- a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js +++ b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js @@ -65,14 +65,16 @@ QUnit.test('the default resolver looks up templates in Ember.TEMPLATES', functio setTemplate('fooBar', fooBarTemplate); setTemplate('fooBar/baz', fooBarBazTemplate); + ignoreDeprecation(() => { + equal(locator.lookupFactory('template:foo'), fooTemplate, 'resolves template:foo'); + equal(locator.lookupFactory('template:fooBar'), fooBarTemplate, 'resolves template:foo_bar'); + equal(locator.lookupFactory('template:fooBar.baz'), fooBarBazTemplate, 'resolves template:foo_bar.baz'); + }); + if (isFeatureEnabled('ember-factory-for')) { equal(locator.factoryFor('template:foo').class, fooTemplate, 'resolves template:foo'); equal(locator.factoryFor('template:fooBar').class, fooBarTemplate, 'resolves template:foo_bar'); equal(locator.factoryFor('template:fooBar.baz').class, fooBarBazTemplate, 'resolves template:foo_bar.baz'); - } else { - equal(locator.lookupFactory('template:foo'), fooTemplate, 'resolves template:foo'); - equal(locator.lookupFactory('template:fooBar'), fooBarTemplate, 'resolves template:foo_bar'); - equal(locator.lookupFactory('template:fooBar.baz'), fooBarBazTemplate, 'resolves template:foo_bar.baz'); } }); @@ -93,48 +95,61 @@ QUnit.test('the default resolver looks up arbitrary types on the namespace', fun QUnit.test('the default resolver resolves models on the namespace', function() { application.Post = EmberObject.extend({}); + ignoreDeprecation(() => { + detectEqual(application.Post, locator.lookupFactory('model:post'), 'looks up Post model on application'); + }); if (isFeatureEnabled('ember-factory-for')) { detectEqual(application.Post, locator.factoryFor('model:post').class, 'looks up Post model on application'); - } else { - detectEqual(application.Post, locator.lookupFactory('model:post'), 'looks up Post model on application'); } }); QUnit.test('the default resolver resolves *:main on the namespace', function() { application.FooBar = EmberObject.extend({}); + ignoreDeprecation(() => { + detectEqual(application.FooBar, locator.lookupFactory('foo-bar:main'), 'looks up FooBar type without name on application'); + }); if (isFeatureEnabled('ember-factory-for')) { detectEqual(application.FooBar, locator.factoryFor('foo-bar:main').class, 'looks up FooBar type without name on application'); - } else { - detectEqual(application.FooBar, locator.lookupFactory('foo-bar:main'), 'looks up FooBar type without name on application'); } }); -QUnit.test('the default resolver resolves container-registered helpers', function() { +if (isFeatureEnabled('ember-factory-for')) { + QUnit.test('the default resolver resolves container-registered helpers', function() { + let shorthandHelper = makeHelper(() => {}); + let helper = Helper.extend(); + + application.register('helper:shorthand', shorthandHelper); + application.register('helper:complete', helper); + + let lookedUpShorthandHelper = locator.factoryFor('helper:shorthand').class; + + ok(lookedUpShorthandHelper.isHelperInstance, 'shorthand helper isHelper'); + + let lookedUpHelper = locator.factoryFor('helper:complete').class; + + ok(lookedUpHelper.isHelperFactory, 'complete helper is factory'); + ok(helper.detect(lookedUpHelper), 'looked up complete helper'); + }); +} + +QUnit.test('the default resolver resolves container-registered helpers via lookupFor', function() { let shorthandHelper = makeHelper(() => {}); let helper = Helper.extend(); application.register('helper:shorthand', shorthandHelper); application.register('helper:complete', helper); - let lookedUpShorthandHelper; - if (isFeatureEnabled('ember-factory-for')) { - lookedUpShorthandHelper = locator.factoryFor('helper:shorthand').class; - } else { - lookedUpShorthandHelper = locator.lookupFactory('helper:shorthand'); - } + ignoreDeprecation(() => { + let lookedUpShorthandHelper = locator.lookupFactory('helper:shorthand'); - ok(lookedUpShorthandHelper.isHelperInstance, 'shorthand helper isHelper'); + ok(lookedUpShorthandHelper.isHelperInstance, 'shorthand helper isHelper'); - let lookedUpHelper; - if (isFeatureEnabled('ember-factory-for')) { - lookedUpHelper = locator.factoryFor('helper:complete').class; - } else { - lookedUpHelper = locator.lookupFactory('helper:complete'); - } + let lookedUpHelper = locator.lookupFactory('helper:complete'); - ok(lookedUpHelper.isHelperFactory, 'complete helper is factory'); - ok(helper.detect(lookedUpHelper), 'looked up complete helper'); + ok(lookedUpHelper.isHelperFactory, 'complete helper is factory'); + ok(helper.detect(lookedUpHelper), 'looked up complete helper'); + }); }); QUnit.test('the default resolver resolves helpers on the namespace', function() { diff --git a/packages/ember-routing/tests/system/controller_for_test.js b/packages/ember-routing/tests/system/controller_for_test.js index 82f1fe27cc8..6eee104e765 100644 --- a/packages/ember-routing/tests/system/controller_for_test.js +++ b/packages/ember-routing/tests/system/controller_for_test.js @@ -7,6 +7,7 @@ import { import controllerFor from '../../system/controller_for'; import generateController from '../../system/generate_controller'; import { buildOwner } from 'internal-test-helpers'; +import { isFeatureEnabled } from 'ember-metal'; function buildInstance(namespace) { let owner = buildOwner(); @@ -75,18 +76,33 @@ QUnit.module('Ember.generateController', { } }); -QUnit.test('generateController should create Ember.Controller', function() { +QUnit.test('generateController should return Ember.Controller', function() { let controller = generateController(appInstance, 'home'); - ok(controller instanceof Controller, 'should create controller'); + ok(controller instanceof Controller, 'should return controller'); }); -QUnit.test('generateController should create App.Controller if provided', function() { +QUnit.test('generateController should return App.Controller if provided', function() { let controller; namespace.Controller = Controller.extend(); controller = generateController(appInstance, 'home'); - ok(controller instanceof namespace.Controller, 'should create controller'); + ok(controller instanceof namespace.Controller, 'should return controller'); +}); + +QUnit.test('generateController should return controller:basic if provided', function() { + let controller; + + let BasicController = Controller.extend(); + appInstance.register('controller:basic', BasicController); + + controller = generateController(appInstance, 'home'); + + if (isFeatureEnabled('ember-no-double-extend')) { + ok(controller instanceof BasicController, 'should return base class of controller'); + } else { + ok(controller instanceof appInstance._lookupFactory('controller:basic'), 'should return double-extended controller'); + } }); diff --git a/tests/node/helpers/build-owner.js b/tests/node/helpers/build-owner.js index 36d03dc2bd4..fa71e9f49ca 100644 --- a/tests/node/helpers/build-owner.js +++ b/tests/node/helpers/build-owner.js @@ -1,6 +1,6 @@ module.exports = function buildOwner(Ember, resolver) { var FACTORY_FOR = Ember.Container.__FACTORY_FOR__; - var LOOKUP_FACTORY = Ember.Container.LOOKUP_FACTORY; + var LOOKUP_FACTORY = Ember.Container.__LOOKUP_FACTORY__; var Owner = Ember.Object.extend(Ember._RegistryProxyMixin, Ember._ContainerProxyMixin, { [FACTORY_FOR]() { return this.__container__[FACTORY_FOR](...arguments);