Skip to content

Commit ffd9314

Browse files
committed
Disabled model factory injections by default.
Given that it is currently very common to use models as globals, and that `App.Post !== lookupFactory('model:post')` when factory injections are enabled, it made sense to disable model factory injections by default. Added the flag Ember.ENV.MODEL_FACTORY_INJECTIONS to enable model injections. Model factory injections will become the default when accessing global models is no longer a common pattern.
1 parent 3f1ef8f commit ffd9314

File tree

6 files changed

+38
-10
lines changed

6 files changed

+38
-10
lines changed

packages/container/lib/main.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
/**
2+
Flag to enable/disable model factory injections (disabled by default)
3+
If model factory injections are enabled, models should not be
4+
accessed globally (only through `container.lookupFactory('model:modelName'))`);
5+
*/
6+
Ember.MODEL_FACTORY_INJECTIONS = false || !!Ember.ENV.MODEL_FACTORY_INJECTIONS;
7+
18
define("container",
29
[],
310
function() {
@@ -359,7 +366,7 @@ define("container",
359366
*/
360367
makeToString: function(factory, fullName) {
361368
return factory.toString();
362-
},
369+
},
363370

364371
/**
365372
Given a fullName return a corresponding instance.
@@ -767,14 +774,15 @@ define("container",
767774
var factory = container.resolve(name);
768775
var injectedFactory;
769776
var cache = container.factoryCache;
777+
var type = fullName.split(":")[0];
770778

771779
if (!factory) { return; }
772780

773781
if (cache.has(fullName)) {
774782
return cache.get(fullName);
775783
}
776784

777-
if (typeof factory.extend !== 'function') {
785+
if (typeof factory.extend !== 'function' || (!Ember.MODEL_FACTORY_INJECTIONS && type === 'model')) {
778786
// TODO: think about a 'safe' merge style extension
779787
// for now just fallback to create time injection
780788
return factory;

packages/container/tests/container_test.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,16 @@ var o_create = Object.create || (function(){
2121
};
2222
}());
2323

24-
module("Container");
24+
var originalModelInjections;
25+
26+
module("Container", {
27+
setup: function() {
28+
originalModelInjections = Ember.MODEL_FACTORY_INJECTIONS;
29+
},
30+
teardown: function() {
31+
Ember.MODEL_FACTORY_INJECTIONS = originalModelInjections;
32+
}
33+
});
2534

2635
var guids = 0;
2736

@@ -331,6 +340,8 @@ test("A failed lookup returns undefined", function() {
331340
});
332341

333342
test("Injecting a failed lookup raises an error", function() {
343+
Ember.MODEL_FACTORY_INJECTIONS = true;
344+
334345
var container = new Container();
335346

336347
var fooInstance = {};

packages/ember-application/tests/system/dependency_injection/to_string_test.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
var originalLookup, App;
1+
var originalLookup, App, originalModelInjections;
22

3-
module("Ember.Application Depedency Injection – toString",{
3+
module("Ember.Application Dependency Injection – toString",{
44
setup: function() {
5+
originalModelInjections = Ember.MODEL_FACTORY_INJECTIONS;
6+
Ember.MODEL_FACTORY_INJECTIONS = true;
7+
58
originalLookup = Ember.lookup;
69

710
Ember.run(function(){
@@ -12,11 +15,13 @@ module("Ember.Application Depedency Injection – toString",{
1215
});
1316

1417
App.Post = Ember.Object.extend();
18+
1519
},
1620

1721
teardown: function() {
1822
Ember.lookup = originalLookup;
1923
Ember.run(App, 'destroy');
24+
Ember.MODEL_FACTORY_INJECTIONS = originalModelInjections;
2025
}
2126
});
2227

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
var locator, originalLookup = Ember.lookup, lookup,
22
application, set = Ember.set, get = Ember.get,
3-
forEach = Ember.ArrayPolyfills.forEach;
3+
forEach = Ember.ArrayPolyfills.forEach, originalModelInjections;
44

5-
module("Ember.Application Depedency Injection", {
5+
module("Ember.Application Dependency Injection", {
66
setup: function() {
7+
originalModelInjections = Ember.MODEL_FACTORY_INJECTIONS;
8+
Ember.MODEL_FACTORY_INJECTIONS = true;
9+
710
application = Ember.run(Ember.Application, 'create');
811

912
application.Person = Ember.Object.extend({});
@@ -26,6 +29,7 @@ module("Ember.Application Depedency Injection", {
2629
Ember.run(application, 'destroy');
2730
application = locator = null;
2831
Ember.lookup = originalLookup;
32+
Ember.MODEL_FACTORY_INJECTIONS = originalModelInjections;
2933
}
3034
});
3135

packages/ember-routing/lib/system/route.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ Ember.Route = Ember.Object.extend(Ember.ActionHandler, {
555555
if (!name && sawParams) { return params; }
556556
else if (!name) { return; }
557557

558-
var modelClass = this.container.lookupFactory('model:' + name).superclass;
558+
var modelClass = this.container.lookupFactory('model:' + name);
559559
var namespace = get(this, 'router.namespace');
560560

561561
Ember.assert("You used the dynamic segment " + name + "_id in your route, but " + namespace + "." + classify(name) + " did not exist and you did not override your route's `model` hook.", modelClass);
@@ -585,7 +585,7 @@ Ember.Route = Ember.Object.extend(Ember.ActionHandler, {
585585
```
586586
587587
The default `serialize` method will insert the model's `id` into the
588-
route's dynamic segment (in this case, `:post_id`) if the segment contains '_id'.
588+
route's dynamic segment (in this case, `:post_id`) if the segment contains '_id'.
589589
If the route has multiple dynamic segments or does not contain '_id', `serialize`
590590
will return `Ember.getProperties(model, params)`
591591

packages/ember-routing/tests/system/route_test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ test("default model utilizes the container to acquire the model factory", functi
3535
function lookupFactory(fullName) {
3636
equal(fullName, "model:post", "correct factory was looked up");
3737

38-
return Post.extend();
38+
return Post;
3939
}
4040

4141
});

0 commit comments

Comments
 (0)