From abc56d5bb3a63b3d23d859c871ac836143e5f327 Mon Sep 17 00:00:00 2001 From: Bob Bijvoet Date: Wed, 24 Feb 2016 16:56:22 +0100 Subject: [PATCH 1/3] feat($compile): add $onDestroy directive lifecycle hook Introduce a destroy hook for directives which follows the ng2 component lifecycle Closes #14020 --- src/ng/compile.js | 5 +++++ test/ng/compileSpec.js | 50 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index bd32ce93ada9..0433517d0d0d 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -296,6 +296,8 @@ * * `$onInit` - Called on each controller after all the controllers on an element have been constructed and * had their bindings initialized (and before the pre & post linking functions for the directives on * this element). This is a good place to put initialization code for your controller. + * * `$onDestroy` - Called when the controller's scope has been destroyed. This is a good place to put + * code for tearing down your controller and for example removing event listeners. * * #### `require` * Require another directive and inject its controller as the fourth argument to the linking function. The @@ -2424,6 +2426,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (isFunction(controller.instance.$onInit)) { controller.instance.$onInit(); } + if (isFunction(controller.instance.$onDestroy)) { + scope.$on('$destroy', controller.instance.$onDestroy); + } }); // PRELINKING diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 6ad592f91ef5..db3d9e49e038 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4584,6 +4584,7 @@ describe('$compile', function() { "class Foo {\n" + " constructor($scope) {}\n" + " $onInit() { this.check(); }\n" + + " $onDestroy() {}\n" + " check() {\n" + " expect(this.data).toEqualData({\n" + " 'foo': 'bar',\n" + @@ -4599,6 +4600,7 @@ describe('$compile', function() { " }\n" + "}"); spyOn(Controller.prototype, '$onInit').andCallThrough(); + spyOn(Controller.prototype, '$onDestroy').andCallThrough(); module(function($compileProvider) { $compileProvider.directive('fooDir', valueFn({ @@ -4625,7 +4627,11 @@ describe('$compile', function() { 'dir-str="Hello, {{whom}}!" ' + 'dir-fn="fn()">')($rootScope); expect(Controller.prototype.$onInit).toHaveBeenCalled(); + expect(Controller.prototype.$onDestroy).not.toHaveBeenCalled(); expect(controllerCalled).toBe(true); + $rootScope.$destroy(); + expect(Controller.prototype.$onDestroy).toHaveBeenCalled(); + }); /*jshint +W061 */ }); @@ -5351,6 +5357,29 @@ describe('$compile', function() { }); }); + it('should call `controller.$onDestroy`, if provided when the controllers is destroyed', function() { + + function Controller1($element) { this.id = 1; this.element = $element; } + Controller1.prototype.$onDestroy = function() {}; + spyOn(Controller1.prototype, '$onDestroy').andCallThrough(); + + function Controller2($element) { this.id = 2; this.element = $element; } + Controller2.prototype.$onDestroy = function() {}; + spyOn(Controller2.prototype, '$onDestroy').andCallThrough(); + + angular.module('my', []) + .directive('d1', valueFn({ controller: Controller1 })) + .directive('d2', valueFn({ controller: Controller2 })); + + module('my'); + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + $rootScope.$destroy(); + expect(Controller1.prototype.$onDestroy).toHaveBeenCalledOnce(); + expect(Controller2.prototype.$onDestroy).toHaveBeenCalledOnce(); + }); + }); + describe('should not overwrite @-bound property each digest when not present', function() { it('when creating new scope', function() { module(function($compileProvider) { @@ -5741,6 +5770,8 @@ describe('$compile', function() { siblingController = this.friend; }; spyOn(MeController.prototype, '$onInit').andCallThrough(); + MeController.prototype.$onDestroy = function() {}; + spyOn(MeController.prototype, '$onDestroy').andCallThrough(); angular.module('my', []) .directive('me', function() { @@ -5770,8 +5801,12 @@ describe('$compile', function() { inject(function($compile, $rootScope, meDirective) { element = $compile('')($rootScope); expect(MeController.prototype.$onInit).toHaveBeenCalled(); + expect(MeController.prototype.$onDestroy).not.toHaveBeenCalled(); expect(parentController).toEqual(jasmine.any(ParentController)); expect(siblingController).toEqual(jasmine.any(SiblingController)); + $rootScope.$destroy(); + expect(MeController.prototype.$onDestroy).toHaveBeenCalled(); + }); }); @@ -5788,6 +5823,9 @@ describe('$compile', function() { }; spyOn(MeController.prototype, '$onInit').andCallThrough(); + MeController.prototype.$onDestroy = function() {}; + spyOn(MeController.prototype, '$onDestroy').andCallThrough(); + angular.module('my', []) .directive('me', function() { return { @@ -5814,8 +5852,12 @@ describe('$compile', function() { inject(function($compile, $rootScope, meDirective) { element = $compile('')($rootScope); expect(MeController.prototype.$onInit).toHaveBeenCalled(); + expect(MeController.prototype.$onDestroy).not.toHaveBeenCalled(); expect(parentController).toBeUndefined(); expect(siblingController).toBeUndefined(); + $rootScope.$destroy(); + expect(MeController.prototype.$onDestroy).toHaveBeenCalled(); + }); }); @@ -5830,9 +5872,12 @@ describe('$compile', function() { $onInit: function() { parentController = this.container; siblingController = this.friend; - } + }, + $onDestroy: function() {} }; spyOn(meController, '$onInit').andCallThrough(); + spyOn(meController, '$onDestroy').andCallThrough(); + return meController; } @@ -5864,8 +5909,11 @@ describe('$compile', function() { inject(function($compile, $rootScope, meDirective) { element = $compile('')($rootScope); expect(meController.$onInit).toHaveBeenCalled(); + expect(meController.$onDestroy).not.toHaveBeenCalled(); expect(parentController).toEqual(jasmine.any(ParentController)); expect(siblingController).toEqual(jasmine.any(SiblingController)); + $rootScope.$destroy(); + expect(meController.$onDestroy).toHaveBeenCalled(); }); }); From 1db0801b9978deba855dbd027fd6aa13903f21a6 Mon Sep 17 00:00:00 2001 From: Bob Bijvoet Date: Thu, 25 Feb 2016 09:20:34 +0100 Subject: [PATCH 2/3] feat($compile): add $onDestroy directive lifecycle hook Bind $onDestroy handler to the controller instance Closes #14020 --- src/ng/compile.js | 2 +- test/ng/compileSpec.js | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 0433517d0d0d..f2b5a3603d5f 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2427,7 +2427,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { controller.instance.$onInit(); } if (isFunction(controller.instance.$onDestroy)) { - scope.$on('$destroy', controller.instance.$onDestroy); + scope.$on('$destroy', controller.instance.$onDestroy.bind(controller.instance)); } }); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index db3d9e49e038..924c2157ad20 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -5359,13 +5359,16 @@ describe('$compile', function() { it('should call `controller.$onDestroy`, if provided when the controllers is destroyed', function() { + function check() { + /*jshint validthis:true */ + expect(this.element.controller('d1').id).toEqual(1); + expect(this.element.controller('d2').id).toEqual(2); + } function Controller1($element) { this.id = 1; this.element = $element; } - Controller1.prototype.$onDestroy = function() {}; - spyOn(Controller1.prototype, '$onDestroy').andCallThrough(); + Controller1.prototype.$onDestroy = jasmine.createSpy('$onDestroy').andCallFake(check); function Controller2($element) { this.id = 2; this.element = $element; } - Controller2.prototype.$onDestroy = function() {}; - spyOn(Controller2.prototype, '$onDestroy').andCallThrough(); + Controller2.prototype.$onDestroy = jasmine.createSpy('$onDestroy').andCallFake(check); angular.module('my', []) .directive('d1', valueFn({ controller: Controller1 })) From 341c7ecf54933916f26f5e97ab174857a1659124 Mon Sep 17 00:00:00 2001 From: Bob Bijvoet Date: Thu, 25 Feb 2016 16:00:33 +0100 Subject: [PATCH 3/3] feat($compile): add $onDestroy directive lifecycle hook Call $onDestroy when element is removed, possibility to set $onDestroy method after controller initialization Closes #14020 --- src/ng/compile.js | 16 +++++++----- test/ng/compileSpec.js | 59 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index f2b5a3603d5f..1ac80256ae80 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -296,8 +296,8 @@ * * `$onInit` - Called on each controller after all the controllers on an element have been constructed and * had their bindings initialized (and before the pre & post linking functions for the directives on * this element). This is a good place to put initialization code for your controller. - * * `$onDestroy` - Called when the controller's scope has been destroyed. This is a good place to put - * code for tearing down your controller and for example removing event listeners. + * * `$onDestroy` - Called on each controller when the directive has been destroyed. This is a good place to put + * code for tearing down your controller like for example removing event listeners. * * #### `require` * Require another directive and inject its controller as the fourth argument to the linking function. The @@ -2421,14 +2421,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } }); - // Trigger the `$onInit` method on all controllers that have one + // Trigger the `$onInit` method on all controllers that have one, + // and trigger `$onDestroy` method if present and when the element emits `$destroy` event forEach(elementControllers, function(controller) { if (isFunction(controller.instance.$onInit)) { controller.instance.$onInit(); } - if (isFunction(controller.instance.$onDestroy)) { - scope.$on('$destroy', controller.instance.$onDestroy.bind(controller.instance)); - } + + $element.on('$destroy', function() { + if (isFunction(controller.instance.$onDestroy)) { + controller.instance.$onDestroy(); + } + }); }); // PRELINKING diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 924c2157ad20..9f022061d7db 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4629,7 +4629,7 @@ describe('$compile', function() { expect(Controller.prototype.$onInit).toHaveBeenCalled(); expect(Controller.prototype.$onDestroy).not.toHaveBeenCalled(); expect(controllerCalled).toBe(true); - $rootScope.$destroy(); + element.remove(); expect(Controller.prototype.$onDestroy).toHaveBeenCalled(); }); @@ -5357,7 +5357,7 @@ describe('$compile', function() { }); }); - it('should call `controller.$onDestroy`, if provided when the controllers is destroyed', function() { + it('should call `controller.$onDestroy`, if provided when the element is removed', function() { function check() { /*jshint validthis:true */ @@ -5377,12 +5377,58 @@ describe('$compile', function() { module('my'); inject(function($compile, $rootScope) { element = $compile('
')($rootScope); - $rootScope.$destroy(); + element.remove(); expect(Controller1.prototype.$onDestroy).toHaveBeenCalledOnce(); expect(Controller2.prototype.$onDestroy).toHaveBeenCalledOnce(); }); }); + it('should call `controller.$onDestroy`, if provided when the directive is removed using ngIf', function() { + + function check() { + /*jshint validthis:true */ + expect(this.element.controller('d1').id).toEqual(1); + } + + function Controller1($element) { this.id = 1; this.element = $element; } + Controller1.prototype.$onDestroy = jasmine.createSpy('$onDestroy').andCallFake(check); + + angular.module('my', []) + .directive('d1', valueFn({ controller: Controller1 })); + + module('my'); + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + $rootScope.t = true; + $rootScope.$apply(); + + $rootScope.t = false; + $rootScope.$apply(); + + expect(Controller1.prototype.$onDestroy).toHaveBeenCalledOnce(); + }); + }); + + it('should call `controller.$onDestroy`, when provided after controller initialization', function() { + + function Controller1() { + this.setDestroy = function() { + Controller1.prototype.$onDestroy = jasmine.createSpy('$onDestroy'); + }; + } + + angular.module('my', []) + .directive('d1', valueFn({ controller: Controller1 })); + + module('my'); + inject(function($compile, $rootScope) { + element = $compile('
')($rootScope); + element.controller('d1').setDestroy(); + element.remove(); + expect(Controller1.prototype.$onDestroy).toHaveBeenCalledOnce(); + }); + }); + describe('should not overwrite @-bound property each digest when not present', function() { it('when creating new scope', function() { module(function($compileProvider) { @@ -5807,7 +5853,7 @@ describe('$compile', function() { expect(MeController.prototype.$onDestroy).not.toHaveBeenCalled(); expect(parentController).toEqual(jasmine.any(ParentController)); expect(siblingController).toEqual(jasmine.any(SiblingController)); - $rootScope.$destroy(); + element.remove(); expect(MeController.prototype.$onDestroy).toHaveBeenCalled(); }); @@ -5825,7 +5871,6 @@ describe('$compile', function() { siblingController = this.friend; }; spyOn(MeController.prototype, '$onInit').andCallThrough(); - MeController.prototype.$onDestroy = function() {}; spyOn(MeController.prototype, '$onDestroy').andCallThrough(); @@ -5858,7 +5903,7 @@ describe('$compile', function() { expect(MeController.prototype.$onDestroy).not.toHaveBeenCalled(); expect(parentController).toBeUndefined(); expect(siblingController).toBeUndefined(); - $rootScope.$destroy(); + element.remove(); expect(MeController.prototype.$onDestroy).toHaveBeenCalled(); }); @@ -5915,7 +5960,7 @@ describe('$compile', function() { expect(meController.$onDestroy).not.toHaveBeenCalled(); expect(parentController).toEqual(jasmine.any(ParentController)); expect(siblingController).toEqual(jasmine.any(SiblingController)); - $rootScope.$destroy(); + element.remove(); expect(meController.$onDestroy).toHaveBeenCalled(); }); });