Introduce ember-registry-container-reform feature flag.

* Ensure that `Container#_registry` remains supported for now. Can be deleted once cont/reg reform is enabled by default. * Continue to expose `registry` and `container` on `ApplicationInstance`. These can be removed once cont/reg reform is enabled, but we’ll still need to expose `ApplicationInstance.container.lookup` until 3.0. * Flag deprecation of Application initializer `initialize` arguments.
emberjs · Jul 23, 2015 · c353c38 · c353c38
1 parent 253edc4
commit c353c38
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 45 deletions.
diff --git a/FEATURES.md b/FEATURES.md
@@ -325,3 +325,15 @@ for a detailed explanation.
 
   Implemencts RFC https://github.com/emberjs/rfcs/pull/65, adding support for
   custom deprecation and warning handlers.
+
+* `ember-registry-container-reform`
+
+  Implements RFC https://github.com/emberjs/rfcs/pull/46, fully encapsulating
+  and privatizing the `Container` and `Registry` classes by exposing a select
+  subset of public methods on `Application` and `ApplicationInstance`.
+
+  `Application` initializers now receive a single argument to `initialize`:
+  `application`.
+
+  Likewise, `ApplicationInstance` initializers still receive a single argument
+  to initialize: `applicationInstance`.
diff --git a/features.json b/features.json
@@ -22,7 +22,8 @@
     "ember-htmlbars-get-helper": true,
     "ember-htmlbars-helper": true,
     "ember-htmlbars-dashless-helpers": true,
-    "ember-debug-handlers": null
+    "ember-debug-handlers": null,
+    "ember-registry-container-reform": null
   },
   "debugStatements": [
     "Ember.warn",

diff --git a/packages/container/lib/container.js b/packages/container/lib/container.js
@@ -1,5 +1,6 @@
 import Ember from 'ember-metal/core'; // Ember.assert
 import dictionary from 'ember-metal/dictionary';
+import isEnabled from 'ember-metal/features';
 
 /**
  A container used to instantiate and cache objects.
@@ -331,4 +332,16 @@ function resetMember(container, fullName) {
   }
 }
 
+// Once registry / container reform is enabled, we no longer need to expose
+// Container#_registry, since Container itself will be fully private.
+if (!isEnabled('ember-registry-container-reform')) {
+  Object.defineProperty(Container, '_registry', {
+    configurable: true,
+    enumerable: false,
+    get() {
+      return this.registry;
+    }
+  });
+}
+
 export default Container;
diff --git a/packages/ember-application/lib/system/application-instance.js b/packages/ember-application/lib/system/application-instance.js
@@ -4,6 +4,8 @@
 @private
 */
 
+import Ember from 'ember-metal'; // Ember.deprecate
+import isEnabled from 'ember-metal/features';
 import { get } from 'ember-metal/property_get';
 import { set } from 'ember-metal/property_set';
 import EmberObject from 'ember-runtime/system/object';
@@ -36,7 +38,7 @@ import ContainerProxy from 'ember-runtime/mixins/container_proxy';
   @public
 */
 
-export default EmberObject.extend(RegistryProxy, ContainerProxy, {
+let ApplicationInstance = EmberObject.extend(RegistryProxy, ContainerProxy, {
   /**
     The `Application` for which this is an instance.
 
@@ -201,3 +203,39 @@ export default EmberObject.extend(RegistryProxy, ContainerProxy, {
 function isResolverModuleBased(applicationInstance) {
   return !!applicationInstance.application.__registry__.resolver.moduleBasedResolver;
 }
+
+if (isEnabled('ember-registry-container-reform')) {
+  Object.defineProperty(ApplicationInstance, 'container', {
+    configurable: true,
+    enumerable: false,
+    get() {
+      var instance = this;
+      return {
+        lookup() {
+          Ember.deprecate('Using `ApplicationInstance.container.lookup` is deprecated. Please use `ApplicationInstance.lookup` instead.',
+                          false,
+                          { id: 'ember-application.app-instance-container', until: '3.0.0' });
+          return instance.lookup(...arguments);
+        }
+      };
+    }
+  });
+} else {
+  Object.defineProperty(ApplicationInstance, 'container', {
+    configurable: true,
+    enumerable: false,
+    get() {
+      return this.__container__;
+    }
+  });
+
+  Object.defineProperty(ApplicationInstance, 'registry', {
+    configurable: true,
+    enumerable: false,
+    get() {
+      return this.__registry__;
+    }
+  });
+}
+
+export default ApplicationInstance;
diff --git a/packages/ember-application/lib/system/application.js b/packages/ember-application/lib/system/application.js
@@ -573,7 +573,11 @@ var Application = Namespace.extend(RegistryProxy, {
     this._runInitializer('initializers', function(name, initializer) {
       Ember.assert('No application initializer named \'' + name + '\'', !!initializer);
       if (initializer.initialize.length === 2) {
-        Ember.deprecate('The `initialize` method for Application initializer \'' + name + '\' should take only one argument - `App`, an instance of an `Application`.');
+        if (isEnabled('ember-registry-container-reform')) {
+          Ember.deprecate('The `initialize` method for Application initializer \'' + name + '\' should take only one argument - `App`, an instance of an `Application`.',
+                          false,
+                          { id: 'ember-application.app-initializer-initialize-arguments', until: '3.0.0' });
+        }
         initializer.initialize(App.__registry__, App);
       } else {
         initializer.initialize(App);

diff --git a/packages/ember-application/tests/system/initializers_test.js b/packages/ember-application/tests/system/initializers_test.js
@@ -2,6 +2,8 @@ import Ember from 'ember-metal/core';
 import run from 'ember-metal/run_loop';
 import Application from 'ember-application/system/application';
 import jQuery from 'ember-views/system/jquery';
+import Registry from 'container/registry';
+import isEnabled from 'ember-metal/features';
 
 var app;
 
@@ -32,23 +34,44 @@ QUnit.test('initializers require proper \'name\' and \'initialize\' properties',
   });
 });
 
-QUnit.test('initializers are passed an App', function() {
-  var MyApplication = Application.extend();
+if (isEnabled('ember-registry-container-reform')) {
+  QUnit.test('initializers are passed an App', function() {
+    var MyApplication = Application.extend();
 
-  MyApplication.initializer({
-    name: 'initializer',
-    initialize(App) {
-      ok(App instanceof Application, 'initialize is passed an Application');
-    }
+    MyApplication.initializer({
+      name: 'initializer',
+      initialize(App) {
+        ok(App instanceof Application, 'initialize is passed an Application');
+      }
+    });
+
+    run(function() {
+      app = MyApplication.create({
+        router: false,
+        rootElement: '#qunit-fixture'
+      });
+    });
   });
+} else {
+  QUnit.test('initializers are passed a registry and App', function() {
+    var MyApplication = Application.extend();
 
-  run(function() {
-    app = MyApplication.create({
-      router: false,
-      rootElement: '#qunit-fixture'
+    MyApplication.initializer({
+      name: 'initializer',
+      initialize(registry, App) {
+        ok(registry instanceof Registry, 'initialize is passed a registry');
+        ok(App instanceof Application, 'initialize is passed an Application');
+      }
+    });
+
+    run(function() {
+      app = MyApplication.create({
+        router: false,
+        rootElement: '#qunit-fixture'
+      });
     });
   });
-});
+}
 
 QUnit.test('initializers can be registered in a specified order', function() {
   var order = [];
@@ -350,23 +373,25 @@ QUnit.test('initializers should be executed in their own context', function() {
   });
 });
 
-QUnit.test('initializers should throw a deprecation warning when receiving a second argument', function() {
-  expect(1);
+if (isEnabled('ember-registry-container-reform')) {
+  QUnit.test('initializers should throw a deprecation warning when receiving a second argument', function() {
+    expect(1);
 
-  var MyApplication = Application.extend();
+    var MyApplication = Application.extend();
 
-  MyApplication.initializer({
-    name: 'deprecated',
-    initialize(registry, application) {
-    }
-  });
+    MyApplication.initializer({
+      name: 'deprecated',
+      initialize(registry, application) {
+      }
+    });
 
-  expectDeprecation(function() {
-    run(function() {
-      app = MyApplication.create({
-        router: false,
-        rootElement: '#qunit-fixture'
+    expectDeprecation(function() {
+      run(function() {
+        app = MyApplication.create({
+          router: false,
+          rootElement: '#qunit-fixture'
+        });
       });
-    });
-  }, /The `initialize` method for Application initializer 'deprecated' should take only one argument - `App`, an instance of an `Application`./);
-});
+    }, /The `initialize` method for Application initializer 'deprecated' should take only one argument - `App`, an instance of an `Application`./);
+  });
+}
diff --git a/packages/ember-application/tests/system/reset_test.js b/packages/ember-application/tests/system/reset_test.js
@@ -8,6 +8,7 @@ import View from 'ember-views/views/view';
 import Controller from 'ember-runtime/controllers/controller';
 import jQuery from 'ember-views/system/jquery';
 import Registry from 'container/registry';
+import isEnabled from 'ember-metal/features';
 
 var application, Application;
 
@@ -240,15 +241,27 @@ QUnit.test('With ember-data like initializer and constant', function() {
     })
   };
 
-  Application.initializer({
-    name: 'store',
-    initialize(application) {
-      application.unregister('store:main');
-      application.register('store:main', application.Store);
+  if (isEnabled('ember-registry-container-reform')) {
+    Application.initializer({
+      name: 'store',
+      initialize(application) {
+        application.unregister('store:main');
+        application.register('store:main', application.Store);
 
-      application.__container__.lookup('store:main');
-    }
-  });
+        application.__container__.lookup('store:main');
+      }
+    });
+  } else {
+    Application.initializer({
+      name: 'store',
+      initialize(registry, application) {
+        registry.unregister('store:main');
+        registry.register('store:main', application.Store);
+
+        application.__container__.lookup('store:main');
+      }
+    });
+  }
 
   run(function() {
     application = Application.create();

diff --git a/packages/ember-routing/lib/initializers/routing-service.js b/packages/ember-routing/lib/initializers/routing-service.js
@@ -1,14 +1,27 @@
 import { onLoad } from 'ember-runtime/system/lazy_load';
 import RoutingService from 'ember-routing/services/routing';
+import isEnabled from 'ember-metal/features';
+
+let initialize;
+if (isEnabled('ember-registry-container-reform')) {
+  initialize = function(application) {
+    // Register the routing service...
+    application.register('service:-routing', RoutingService);
+    // Then inject the app router into it
+    application.inject('service:-routing', 'router', 'router:main');
+  };
+} else {
+  initialize = function(registry, application) {
+    // Register the routing service...
+    registry.register('service:-routing', RoutingService);
+    // Then inject the app router into it
+    registry.injection('service:-routing', 'router', 'router:main');
+  };
+}
 
 onLoad('Ember.Application', function(Application) {
   Application.initializer({
     name: 'routing-service',
-    initialize(application) {
-      // Register the routing service...
-      application.register('service:-routing', RoutingService);
-      // Then inject the app router into it
-      application.inject('service:-routing', 'router', 'router:main');
-    }
+    initialize
   });
 });