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

Commit e6966e0

Browse files
vojtajinaIgorMinar
authored andcommitted
fix(Scope): allow removing a listener during event
1 parent 682418f commit e6966e0

File tree

2 files changed

+91
-5
lines changed

2 files changed

+91
-5
lines changed

src/ng/rootScope.js

+23-5
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ function $RootScopeProvider(){
638638
namedListeners.push(listener);
639639

640640
return function() {
641-
arrayRemove(namedListeners, listener);
641+
namedListeners[indexOf(namedListeners, listener)] = null;
642642
};
643643
},
644644

@@ -686,6 +686,14 @@ function $RootScopeProvider(){
686686
namedListeners = scope.$$listeners[name] || empty;
687687
event.currentScope = scope;
688688
for (i=0, length=namedListeners.length; i<length; i++) {
689+
690+
// if listeners were deregistered, defragment the array
691+
if (!namedListeners[i]) {
692+
namedListeners.splice(i, 1);
693+
i--;
694+
length--;
695+
continue;
696+
}
689697
try {
690698
namedListeners[i].apply(null, listenerArgs);
691699
if (stopPropagation) return event;
@@ -735,19 +743,29 @@ function $RootScopeProvider(){
735743
},
736744
defaultPrevented: false
737745
},
738-
listenerArgs = concat([event], arguments, 1);
746+
listenerArgs = concat([event], arguments, 1),
747+
listeners, i, length;
739748

740749
//down while you can, then up and next sibling or up and next sibling until back at root
741750
do {
742751
current = next;
743752
event.currentScope = current;
744-
forEach(current.$$listeners[name], function(listener) {
753+
listeners = current.$$listeners[name] || [];
754+
for (i=0, length = listeners.length; i<length; i++) {
755+
// if listeners were deregistered, defragment the array
756+
if (!listeners[i]) {
757+
listeners.splice(i, 1);
758+
i--;
759+
length--;
760+
continue;
761+
}
762+
745763
try {
746-
listener.apply(null, listenerArgs);
764+
listeners[i].apply(null, listenerArgs);
747765
} catch(e) {
748766
$exceptionHandler(e);
749767
}
750-
});
768+
}
751769

752770
// Insanity Warning: scope depth-first traversal
753771
// yes, this code is a bit crazy, but it works and we have tests to prove it!

test/ng/rootScopeSpec.js

+68
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,74 @@ describe('Scope', function() {
708708
});
709709

710710

711+
it('should allow removing event listener inside a listener on $emit', function() {
712+
var spy1 = jasmine.createSpy('1st listener');
713+
var spy2 = jasmine.createSpy('2nd listener');
714+
var spy3 = jasmine.createSpy('3rd listener');
715+
716+
var remove1 = child.$on('evt', spy1);
717+
var remove2 = child.$on('evt', spy2);
718+
var remove3 = child.$on('evt', spy3);
719+
720+
spy1.andCallFake(remove1);
721+
722+
expect(child.$$listeners['evt'].length).toBe(3);
723+
724+
// should call all listeners and remove 1st
725+
child.$emit('evt');
726+
expect(spy1).toHaveBeenCalledOnce();
727+
expect(spy2).toHaveBeenCalledOnce();
728+
expect(spy3).toHaveBeenCalledOnce();
729+
expect(child.$$listeners['evt'].length).toBe(3); // cleanup will happen on next $emit
730+
731+
spy1.reset();
732+
spy2.reset();
733+
spy3.reset();
734+
735+
// should call only 2nd because 1st was already removed and 2nd removes 3rd
736+
spy2.andCallFake(remove3);
737+
child.$emit('evt');
738+
expect(spy1).not.toHaveBeenCalled();
739+
expect(spy2).toHaveBeenCalledOnce();
740+
expect(spy3).not.toHaveBeenCalled();
741+
expect(child.$$listeners['evt'].length).toBe(1);
742+
});
743+
744+
745+
it('should allow removing event listener inside a listener on $broadcast', function() {
746+
var spy1 = jasmine.createSpy('1st listener');
747+
var spy2 = jasmine.createSpy('2nd listener');
748+
var spy3 = jasmine.createSpy('3rd listener');
749+
750+
var remove1 = child.$on('evt', spy1);
751+
var remove2 = child.$on('evt', spy2);
752+
var remove3 = child.$on('evt', spy3);
753+
754+
spy1.andCallFake(remove1);
755+
756+
expect(child.$$listeners['evt'].length).toBe(3);
757+
758+
// should call all listeners and remove 1st
759+
child.$broadcast('evt');
760+
expect(spy1).toHaveBeenCalledOnce();
761+
expect(spy2).toHaveBeenCalledOnce();
762+
expect(spy3).toHaveBeenCalledOnce();
763+
expect(child.$$listeners['evt'].length).toBe(3); //cleanup will happen on next $broadcast
764+
765+
spy1.reset();
766+
spy2.reset();
767+
spy3.reset();
768+
769+
// should call only 2nd because 1st was already removed and 2nd removes 3rd
770+
spy2.andCallFake(remove3);
771+
child.$broadcast('evt');
772+
expect(spy1).not.toHaveBeenCalled();
773+
expect(spy2).toHaveBeenCalledOnce();
774+
expect(spy3).not.toHaveBeenCalled();
775+
expect(child.$$listeners['evt'].length).toBe(1);
776+
});
777+
778+
711779
describe('event object', function() {
712780
it('should have methods/properties', function() {
713781
var event;

0 commit comments

Comments
 (0)