Skip to content
This repository was archived by the owner on Oct 10, 2022. It is now read-only.

Commit dabacfe

Browse files
committed
fix(error message): improve error on ngModule redefinition
- when a user reuses an angular module, i.e. by specifying the same module name in multiple ngDefine calls, redefinition (i.e. adding more `module:some.angular.module` dependencies) is not allowed, as angular does not allow to re-define module definitions - added detailed error message to better reflect that issue Closes #1
1 parent a7e6099 commit dabacfe

File tree

4 files changed

+36
-8
lines changed

4 files changed

+36
-8
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ We will check out how to employ `ngDefine` to [define AngularJS modules](#Angula
4040

4141
### AngularJS Module Definition
4242

43-
AngularJS modules may be declared using `#ngDefine(name, dependencies, callback)`.
43+
AngularJS modules may be declared using `#ngDefine(name[, dependencies], callback)`.
4444
The function accepts the name of the module to be defined or looked up and a callback function that may define or configure the module.
4545
A (optional) list of arbitrary RequireJS dependencies can be passed that are resolved before the callback function is executed.
4646

src/ngDefine.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* @version 1.0.0
55
* @author Nico Rehwaldt <http://github.com/Nikku>
66
*
7-
* @license MIT
7+
* @license (c) 2013 Nico Rehwaldt, MIT
88
*/
99
(function(window) {
1010

@@ -70,7 +70,9 @@
7070
}
7171

7272
if (modules.length && exists) {
73-
throw new Error("Failed to define module " + name + ". Already exists");
73+
throw new Error(
74+
"Cannot re-define angular module " + name + " with new dependencies [" + modules + "]. " +
75+
"Make sure the module is not defined else where or define a sub-module with additional angular module dependencies instead.");
7476
}
7577

7678
if (modules.length || !exists) {
@@ -102,7 +104,7 @@
102104
var angularDefine = function(angular) {
103105
return function(name, dependencies, body) {
104106
if (!dependencies) {
105-
throw new Error("wrong number of arguments, expected name, [dependencies, ]body");
107+
throw new Error("wrong number of arguments, expected name[, dependencies], body");
106108
}
107109
internalModule(angular, name, dependencies, body);
108110
};

test/errorSpec.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
define([ 'ngDefine' ], function() {
2+
3+
return describe('ngDefine()', function() {
4+
5+
describe('should throw error', function() {
6+
7+
it("when redefining angular module with additional dependencies", function() {
8+
9+
var foo, bar;
10+
11+
ngDefine('myerror.module', ['module:ng'], function() {
12+
this.foo = true;
13+
});
14+
15+
expect(function() {
16+
ngDefine('myerror.module', ['module:ng'], function() {
17+
this.bar = true;
18+
});
19+
}).toThrow();
20+
21+
});
22+
});
23+
});
24+
});

test/runner.html

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
<script src="lib/jasmine/jasmine-html.js"></script>
1111

1212
<script src="../lib/require/require.js"></script>
13+
<script src="main.js"></script>
1314

1415
<script>
1516
(function() {
@@ -25,16 +26,16 @@
2526
},
2627
shim: {
2728
'angular' : { deps: [ 'jquery' ], exports: 'angular' },
28-
'angular-resource': { deps: [ 'angular' ] },
29-
'angular-mocks': { deps: [ 'angular' ] },
29+
'angular-resource': { deps: [ 'angular' ] },
30+
'angular-mocks': { deps: [ 'angular' ] },
3031
},
3132
packages: [
3233
{ name: 'test', location: 'test' },
3334
{ name: 'my-module', location: 'test/my-module' },
3435
{ name: 'my-other-module', location: 'test/my-other-module' }
3536
]
3637
});
37-
38+
3839
var jasmineEnv = jasmine.getEnv();
3940
jasmineEnv.updateInterval = 1000;
4041

@@ -47,7 +48,8 @@
4748
};
4849

4950
var specs = [
50-
'test/appSpec'
51+
'test/appSpec',
52+
'test/errorSpec'
5153
];
5254

5355
var testOnly = window.location.hash.substring(1);

0 commit comments

Comments
 (0)