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

Scope destroy + better DI inference #798

Merged
merged 3 commits into from
Mar 16, 2012
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions docs/content/guide/dev_guide.di.understanding_di.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,103 @@ minifiers/obfuscators. In the future, we may provide a pre-processor which will
code and insert the `$inject` into the source code so that it can be minified/obfuscated.


### Dependency inference and variable name shadowing

During inference, the injector considers argument names with leading and trailing underscores to be
equivivalent to the name without these underscores. For example `_fooSvc_` argument name is treated
as if it was `fooSvc`, this is useful especially in tests where variable name shadowing can cause
some friction. This is best illustrated on examples:

When testing a service, it's common to need a reference to it in every single test. This can be
done in jasmine with DI inference like this:

<pre>
describe('fooSvc', function() {
it('should do this thing', inject(function(fooSvc) {
//test fooSvc
}));

it('should do that thing', inject(function(fooSvc) {
//test fooSvc
}));

// more its
});
</pre>

... but having to inject the service over and over gets easily tiresome.

It's likely better to rewrite these tests with a use of jasmine's `beforeEach`:

<pre>
describe('fooSvc', function() {
var fooSvc;

beforeEach(inject(function(fooSvc) {
fooSvc = fooSvc; // DOESN'T WORK! outer fooSvc is being shadowed
}));

it('should do this thing', function() {
//test fooSvc
});

it('should do that thing', function() {
//test fooSvc
});

// more its
});
</pre>

This obviously won't work because `fooSvc` variable in the describe block is being shadowed by the
`fooSvc` argument of the beforeEach function. So we have to resort to alternative solutions, like
for example use of array notation to annotate the beforeEach fn:

<pre>
describe('fooSvc', function() {
var fooSvc;

beforeEach(inject(['fooSvc', function(fooSvc_) {
fooSvc = fooSvc_;
}]));

it('should do this thing', function() {
//test fooSvc
});

it('should do that thing', function() {
//test fooSvc
});
});
</pre>


That's better, but it's still annoying, especially if you have many services to inject.

To resolve this shadowing problem, the injector considers `_fooSvc_` argument names equal to
`fooSvc`, so the test can be rewritten like this:

<pre>
describe('fooSvc', function() {
var fooSvc;

beforeEach(inject(function(_fooSvc_) {
fooSvc = _fooSvc_;
}));

it('should do this thing', function() {
//test fooSvc
});

it('should do that thing', function() {
//test fooSvc
});

// more its
});
</pre>


## Related Topics

* {@link dev_guide.services Angular Services}
Expand Down
4 changes: 2 additions & 2 deletions src/Injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

var FN_ARGS = /^function\s*[^\(]*\(([^\)]*)\)/m;
var FN_ARG_SPLIT = /,/;
var FN_ARG = /^\s*(.+?)\s*$/;
var FN_ARG = /^\s*(_?)(.+?)\1\s*$/;
var STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
function inferInjectionArgs(fn) {
assertArgFn(fn);
Expand All @@ -49,7 +49,7 @@ function inferInjectionArgs(fn) {
var fnText = fn.toString().replace(STRIP_COMMENTS, '');
var argDecl = fnText.match(FN_ARGS);
forEach(argDecl[1].split(FN_ARG_SPLIT), function(arg){
arg.replace(FN_ARG, function(all, name){
arg.replace(FN_ARG, function(all, underscore, name){
args.push(name);
});
});
Expand Down
3 changes: 2 additions & 1 deletion src/service/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ function $RootScopeProvider(){
this.$$phase = this.$parent = this.$$watchers =
this.$$nextSibling = this.$$prevSibling =
this.$$childHead = this.$$childTail = null;
this.$destructor = noop;
this['this'] = this.$root = this;
this.$$asyncQueue = [];
this.$$listeners = {};
Expand Down Expand Up @@ -458,6 +457,8 @@ function $RootScopeProvider(){
if (this.$root == this) return; // we can't remove the root node;
var parent = this.$parent;

this.$broadcast('$destroy');

if (parent.$$childHead == this) parent.$$childHead = this.$$nextSibling;
if (parent.$$childTail == this) parent.$$childTail = this.$$prevSibling;
if (this.$$prevSibling) this.$$prevSibling.$$nextSibling = this.$$nextSibling;
Expand Down
6 changes: 6 additions & 0 deletions test/InjectorSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ describe('injector', function() {
});


it('should strip leading and trailing underscores from arg name during inference', function() {
function beforeEachFn(_foo_) { /* foo = _foo_ */ };
expect(inferInjectionArgs(beforeEachFn)).toEqual(['foo']);
});


it('should handle no arg functions', function() {
function $f_n0() {}
expect(inferInjectionArgs($f_n0)).toEqual([]);
Expand Down
12 changes: 2 additions & 10 deletions test/directive/ngViewSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,7 @@ describe('ng-view', function() {
var createCtrl = function(name) {
return function($scope) {
log.push('init-' + name);
var destroy = $scope.$destroy;
$scope.$destroy = function() {
log.push('destroy-' + name);
destroy.call($scope);
}
$scope.$on('$destroy', function() {log.push('destroy-' + name);});
};
};

Expand Down Expand Up @@ -369,11 +365,7 @@ describe('ng-view', function() {
function createController(name) {
return function($scope) {
log.push('init-' + name);
var destroy = $scope.$destroy;
$scope.$destroy = function() {
log.push('destroy-' + name);
destroy.call($scope);
}
$scope.$on('$destroy', logger('destroy-' + name));
$scope.$on('$routeUpdate', logger('route-update'));
};
}
Expand Down
12 changes: 12 additions & 0 deletions test/service/scopeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

describe('Scope', function() {

beforeEach(module(provideLog));


describe('$root', function() {
it('should point to itself', inject(function($rootScope) {
expect($rootScope.$root).toEqual($rootScope);
Expand Down Expand Up @@ -393,6 +396,15 @@ describe('Scope', function() {
$rootScope.$digest();
expect(log).toEqual('12');
}));


it('should broadcast the $destroy event', inject(function($rootScope, log) {
first.$on('$destroy', log.fn('first'));
first.$new().$on('$destroy', log.fn('first-child'));

first.$destroy();
expect(log).toEqual('first; first-child');
}));
});


Expand Down