Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 0d3b69a

Browse files
committed
fix($injector): throw when factory $get method does not return a value
BREAKING CHANGE: Previously, not returning a value would fail silently, and an application trying to inject the value owuld inject an undefined value, quite possibly leading to a TypeError. Now, the application will fail entirely, and a reason will be given. Closes #4575 Closes #9210
1 parent 39d0b36 commit 0d3b69a

File tree

3 files changed

+65
-8
lines changed

3 files changed

+65
-8
lines changed
+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
@ngdoc error
2+
@name $injector:undef
3+
@fullName Undefined Value
4+
@description
5+
6+
This error results from registering a factory which does not return a value (or whose return value is undefined).
7+
8+
The following is an example of a factory which will throw this error upon injection:
9+
10+
```js
11+
angular.module("badModule", []).
12+
factory("badFactory", function() {
13+
doLotsOfThings();
14+
butDontReturnAValue();
15+
});
16+
```
17+
18+
In order to prevent the error, return a value of some sort, such as an object which exposes an API for working
19+
with the injected object.
20+
21+
```js
22+
angular.module("goodModule", []).
23+
factory("goodFactory", function() {
24+
doLotsOfThings();
25+
butDontReturnAValue();
26+
27+
return {
28+
doTheThing: function methodThatDoesAThing() {
29+
}
30+
};
31+
});
32+
```
33+

src/auto/injector.js

+16-2
Original file line numberDiff line numberDiff line change
@@ -662,15 +662,29 @@ function createInjector(modulesToLoad, strictDi) {
662662
return providerCache[name + providerSuffix] = provider_;
663663
}
664664

665-
function factory(name, factoryFn) { return provider(name, { $get: factoryFn }); }
665+
function enforceReturnValue(name, factory) {
666+
return function enforcedReturnValue() {
667+
var result = instanceInjector.invoke(factory);
668+
if (isUndefined(result)) {
669+
throw $injectorMinErr('undef', "Provider '{0}' must return a value from $get factory method.", name);
670+
}
671+
return result;
672+
};
673+
}
674+
675+
function factory(name, factoryFn, enforce) {
676+
return provider(name, {
677+
$get: enforce !== false ? enforceReturnValue(name, factoryFn) : factoryFn
678+
});
679+
}
666680

667681
function service(name, constructor) {
668682
return factory(name, ['$injector', function($injector) {
669683
return $injector.instantiate(constructor);
670684
}]);
671685
}
672686

673-
function value(name, val) { return factory(name, valueFn(val)); }
687+
function value(name, val) { return factory(name, valueFn(val), false); }
674688

675689
function constant(name, value) {
676690
assertNotHasOwnProperty(name, 'constant');

test/auto/injectorSpec.js

+16-6
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ describe('injector', function() {
4545
// s6
4646

4747

48-
providers('s1', function() { log.push('s1'); }, {$inject: ['s2', 's5', 's6']});
49-
providers('s2', function() { log.push('s2'); }, {$inject: ['s3', 's4', 's5']});
50-
providers('s3', function() { log.push('s3'); }, {$inject: ['s6']});
51-
providers('s4', function() { log.push('s4'); }, {$inject: ['s3', 's5']});
52-
providers('s5', function() { log.push('s5'); });
53-
providers('s6', function() { log.push('s6'); });
48+
providers('s1', function() { log.push('s1'); return {}; }, {$inject: ['s2', 's5', 's6']});
49+
providers('s2', function() { log.push('s2'); return {}; }, {$inject: ['s3', 's4', 's5']});
50+
providers('s3', function() { log.push('s3'); return {}; }, {$inject: ['s6']});
51+
providers('s4', function() { log.push('s4'); return {}; }, {$inject: ['s3', 's5']});
52+
providers('s5', function() { log.push('s5'); return {}; });
53+
providers('s6', function() { log.push('s6'); return {}; });
5454

5555
injector.get('s1');
5656

@@ -983,4 +983,14 @@ describe('strict-di injector', function() {
983983
}).toThrowMinErr('$injector', 'strictdi');
984984
});
985985
});
986+
987+
988+
it('should throw if factory does not return a value', function() {
989+
module(function($provide) {
990+
$provide.factory('$test', function() {});
991+
});
992+
expect(function() {
993+
inject(function($test) {});
994+
}).toThrowMinErr('$injector', 'undef');
995+
});
986996
});

0 commit comments

Comments
 (0)