8000 feat($compile): add $onDestroy directive lifecycle hook by bobbijvoet · Pull Request #14127 · angular/angular.js · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat($compile): add $onDestroy directive lifecycle hook #14127

Closed
wants to merge 3 commits into from
Closed

feat($compile): add $onDestroy directive lifecycle hook #14127

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
abc56d5
feat($compile): add $onDestroy directive lifecycle hook
Feb 24, 2016
1db0801
feat($compile): add $onDestroy directive lifecycle hook
Feb 25, 2016
341c7ec
feat($compile): add $onDestroy directive lifecycle hook
Feb 25, 2016
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
Prev Previous commit
Next Next commit
feat($compile): add $onDestroy directive lifecycle hook 
Bind $onDestroy handler to the controller instance

Closes #14020
  • Loading branch information
Bob Bijvoet committed Feb 25, 2016
commit 1db0801b9978deba855dbd027fd6aa13903f21a6
2 changes: 1 addition & 1 deletion src/ng/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is that scope might be a parent scope that is not directly related to the element.
Maybe we could use $element.on('$destroy', ...) instead (I haven't given it much thought, though).

< 8000 div id="r54100231" class="timeline-comment-group js-minimizable-comment-group js-targetable-element my-0 comment previewable-edit js-task-list-container js-comment review-comment js-minimize-container unminimized-comment">
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. The element.$on solution sounds better to me because we care about the destruction of the component/directive in specific, not about the scope. Thanks for the insight, will create a new solution.

}
});

Expand Down
11 changes: 7 additions & 4 deletions test/ng/compileSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5359,13 +5359,16 @@ describe('$compile', function() {

it('should call `controller.$onDestroy`, if provided when the controllers is destroyed', function() {
915D Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

controllers --> controller
(although, technically, the controller is not destroyed.)


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 }))
Expand Down
0