Refactor Ember.Application.initializer to fix initializers bleeding or not running

packages/ember-application/lib/system/application.js

@@ -569,10 +569,10 @@ var Application = Ember.Application = Ember.Namespace.extend(Ember.DeferredMixin
         container = this.__container__,
         graph = new Ember.DAG(),
         namespace = this,
-        i, initializer;
+        name, initializer;
 
-    for (i=0; i<initializers.length; i++) {
-      initializer = initializers[i];
+    for (name in initializers) {
+      initializer = initializers[name];
       graph.addEdges(initializer.name, initializer.initialize, initializer.before, initializer.after);
     }
 
@@ -677,16 +677,23 @@ var Application = Ember.Application = Ember.Namespace.extend(Ember.DeferredMixin
 });
 
 Ember.Application.reopenClass({
-  concatenatedProperties: ['initializers'],
-  initializers: Ember.A(),
+  initializers: {},
   initializer: function(initializer) {
-    var initializers = get(this, 'initializers');
+    // If this is the first initializer being added to a subclass, we are going to reopen the class
+    // to make sure we have a new `initializers` object, which extends from the parent class' using
+    // prototypal inheritance. Without this, attempting to add initializers to the subclass would
+    // pollute the parent class as well as other subclasses.
+    if (this.superclass.initializers !== undefined && this.superclass.initializers === this.initializers) {
+      this.reopenClass({
+        initializers: Ember.create(this.initializers)
+      });
+    }
 
-    Ember.assert("The initializer '" + initializer.name + "' has already been registered", !Ember.A(initializers).findBy('name', initializers.name));
+    Ember.assert("The initializer '" + initializer.name + "' has already been registered", !this.initializers[initializer.name]);
     Ember.assert("An initializer cannot be registered with both a before and an after", !(initializer.before && initializer.after));
     Ember.assert("An initializer cannot be registered without an initialize function", Ember.canInvoke(initializer, 'initialize'));
 
-    initializers.push(initializer);
+    this.initializers[initializer.name] = initializer;
   },
 
   /**

packages/ember-application/tests/system/initializers_test.js

@@ -3,13 +3,9 @@ var indexOf = Ember.ArrayPolyfills.indexOf;
 
 module("Ember.Application initializers", {
   setup: function() {
-    oldInitializers = Ember.Application.initializers;
-    Ember.Application.initializers = Ember.A();
   },
 
   teardown: function() {
-    Ember.Application.initializers = oldInitializers;
-
     if (app) {
       Ember.run(function() { app.destroy(); });
     }
@@ -18,57 +14,54 @@ module("Ember.Application initializers", {
 
 test("initializers can be registered in a specified order", function() {
   var order = [];
-  Ember.Application.initializer({
+  var Application = Ember.Application.extend();
+  Application.initializer({
     name: 'fourth',
     after: 'third',
     initialize: function(container) {
       order.push('fourth');
     }
   });
 
-  Ember.Application.initializer({
+  Application.initializer({
     name: 'second',
     before: 'third',
     initialize: function(container) {
       order.push('second');
     }
   });
 
-  Ember.Application.initializer({
+  Application.initializer({
     name: 'fifth',
     after: 'fourth',
     initialize: function(container) {
       order.push('fifth');
     }
   });
 
-  Ember.Application.initializer({
+  Application.initializer({
     name: 'first',
     before: 'second',
     initialize: function(container) {
       order.push('first');
     }
   });
 
-  Ember.Application.initializer({
+  Application.initializer({
     name: 'third',
     initialize: function(container) {
       order.push('third');
     }
   });
 
   Ember.run(function() {
-    app = Ember.Application.create({
+    app = Application.create({
       router: false,
       rootElement: '#qunit-fixture'
     });
   });
 
   deepEqual(order, ['first', 'second', 'third', 'fourth', 'fifth']);
-
-  Ember.run(function() {
-    app.destroy();
-  });
 });
 
 test("initializers can have multiple dependencies", function () {
@@ -124,10 +117,6 @@ test("initializers can have multiple dependencies", function () {
   ok(indexOf.call(order, b.name) < indexOf.call(order, c.name), 'b < c');
   ok(indexOf.call(order, b.name) < indexOf.call(order, afterB.name), 'b < afterB');
   ok(indexOf.call(order, c.name) < indexOf.call(order, afterC.name), 'c < afterC');
-
-  Ember.run(function() {
-    app.destroy();
-  });
 });
 
 test("initializers set on Application subclasses should not be shared between apps", function(){
@@ -164,3 +153,56 @@ test("initializers set on Application subclasses should not be shared between ap
   equal(firstInitializerRunCount, 1, 'second initializer only was run');
   equal(secondInitializerRunCount, 1, 'second initializer only was run');
 });
+
+test("initializers are concatenated", function(){
+  var firstInitializerRunCount = 0, secondInitializerRunCount = 0;
+  var FirstApp = Ember.Application.extend();
+  FirstApp.initializer({
+    name: 'first',
+    initialize: function(container) {
+      firstInitializerRunCount++;
+    }
+  });
+
+  var SecondApp = FirstApp.extend();
+  SecondApp.initializer({
+    name: 'second',
+    initialize: function(container) {
+      secondInitializerRunCount++;
+    }
+  });
+
+  Ember.$('#qunit-fixture').html('<div id="first"></div><div id="second"></div>');
+  Ember.run(function() {
+    var firstApp = FirstApp.create({
+      router: false,
+      rootElement: '#qunit-fixture #first'
+    });
+  });
+  equal(firstInitializerRunCount, 1, 'first initializer only was run when base class created');
+  equal(secondInitializerRunCount, 0, 'first initializer only was run when base class created');
+  firstInitializerRunCount = 0;
+  Ember.run(function() {
+    var secondApp = SecondApp.create({
+      router: false,
+      rootElement: '#qunit-fixture #second'
+    });
+  });
+  equal(firstInitializerRunCount, 1, 'first initializer was run when subclass created');
+  equal(secondInitializerRunCount, 1, 'second initializers was run when subclass created');
+});
+
+test("initializers are per-app", function(){
+  expect(0);
+  var FirstApp = Ember.Application.extend();
+  FirstApp.initializer({
+    name: 'shouldNotCollide',
+    initialize: function(container) {}
+  });
+
+  var SecondApp = Ember.Application.extend();
+  SecondApp.initializer({
+    name: 'shouldNotCollide',
+    initialize: function(container) {}
+  });
+});

packages/ember-metal/lib/mixin.js

@@ -137,9 +137,7 @@ function applyConcatenatedProperties(obj, key, value, values) {
       return Ember.makeArray(baseValue).concat(value);
     }
   } else {
-    // Make sure this mixin has its own array so it is not
-    // accidentally mutated by another child's interactions
-    return Ember.makeArray(value).slice();
+    return Ember.makeArray(value);
   }
 }
 

packages/ember-runtime/tests/system/object/extend_test.js

@@ -1,3 +1,5 @@
+var get = Ember.get;
+
 module('Ember.Object.extend');
 
 test('Basic extend', function() {
@@ -51,4 +53,33 @@ test('Overriding a method several layers deep', function() {
   equal(obj.barCnt, 2, 'should invoke both');
 });
 
+test('With concatenatedProperties', function(){
+  var SomeClass = Ember.Object.extend({ things: 'foo', concatenatedProperties: ['things'] });
+  var AnotherClass = SomeClass.extend({ things: 'bar' });
+  var YetAnotherClass = SomeClass.extend({ things: 'baz' });
+  var some = new SomeClass();
+  var another = new AnotherClass();
+  var yetAnother = new YetAnotherClass();
+  deepEqual(some.get('things'), ['foo'], 'base class should have just its value');
+  deepEqual(another.get('things'), ['foo', 'bar'], "subclass should have base class' and it's own");
+  deepEqual(yetAnother.get('things'), ['foo', 'baz'], "subclass should have base class' and it's own");
+});
+
+test('With concatenatedProperties class properties', function(){
+  var SomeClass = Ember.Object.extend();
+  SomeClass.reopenClass({
+    concatenatedProperties: ['things'],
+    things: 'foo'
+  });
+  var AnotherClass = SomeClass.extend();
+  AnotherClass.reopenClass({ things: 'bar' });
+  var YetAnotherClass = SomeClass.extend();
+  YetAnotherClass.reopenClass({ things: 'baz' });
+  var some = new SomeClass();
+  var another = new AnotherClass();
+  var yetAnother = new YetAnotherClass();
+  deepEqual(get(some.constructor, 'things'), ['foo'], 'base class should have just its value');
+  deepEqual(get(another.constructor, 'things'), ['foo', 'bar'], "subclass should have base class' and it's own");
+  deepEqual(get(yetAnother.constructor, 'things'), ['foo', 'baz'], "subclass should have base class' and it's own");
+});