Skip to content

Commit

Permalink
Revert "[BUGFIX] Fix an issue with concatenatedProperties."
Browse files Browse the repository at this point in the history
This reverts commit 0158de0.
  • Loading branch information
stefanpenner committed Oct 4, 2013
1 parent 56e5a22 commit f1ce398
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 38 deletions.
2 changes: 1 addition & 1 deletion packages/ember-application/lib/system/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ Ember.Application.reopenClass({
initializer: function(initializer) {
var initializers = get(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", !initializers.findBy('name', initializers.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'));

Expand Down
34 changes: 0 additions & 34 deletions packages/ember-application/tests/system/initializers_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,37 +130,3 @@ test("initializers can have multiple dependencies", function () {
});
});

test("initializers set on Application subclasses should not be shared between apps", function(){
var firstInitializerRunCount = 0, secondInitializerRunCount = 0;
var FirstApp = Ember.Application.extend();
FirstApp.initializer({
name: 'first',
initialize: function(container) {
firstInitializerRunCount++;
}
});
var SecondApp = Ember.Application.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');
equal(secondInitializerRunCount, 0, 'first initializer only was run');
Ember.run(function() {
var secondApp = SecondApp.create({
router: false,
rootElement: '#qunit-fixture #second'
});
});
equal(firstInitializerRunCount, 1, 'second initializer only was run');
equal(secondInitializerRunCount, 1, 'second initializer only was run');
});
4 changes: 1 addition & 3 deletions packages/ember-metal/lib/mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

3 comments on commit f1ce398

@stefanpenner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we realized this solution was incorrect, and actually broke some scenarios. #3516 is our latest attempt.

@rwjblue
Copy link
Member

@rwjblue rwjblue commented on f1ce398 Oct 5, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit is broke the build, since there is a leaking initializer (registering 'store:main' from here ). Previously, this was benign because you could just register undefined factories, but as of 4e4ec7a that was changed and we raise an error if you attempt to register a factory is undefined.

This is fixed by #3516, but is that ready to merge?

@stefanpenner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3516 is now merged. Sorry got deep into some pairing and forgot..

Please sign in to comment.