Skip to content

Commit

Permalink
Add protection against newer unsubscribe removing older listeners
Browse files Browse the repository at this point in the history
  • Loading branch information
gaearon committed Oct 23, 2015
1 parent 11ab454 commit b7031ce
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
10 changes: 7 additions & 3 deletions src/createStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,16 @@ export default function createStore(reducer, initialState) {
*/
function subscribe(listener) {
listeners.push(listener);
var isSubscribed = true;

return function unsubscribe() {
var index = listeners.indexOf(listener);
if (index !== -1) {
listeners.splice(index, 1);
if (!isSubscribed) {
return;
}

isSubscribed = false;
var index = listeners.indexOf(listener);
listeners.splice(index, 1);
};
}

Expand Down
16 changes: 15 additions & 1 deletion test/createStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ describe('createStore', () => {
expect(listenerB.calls.length).toBe(2);
});

it('should only remove relevant listener no matter how many times unsubscribe called', () => {
it('should only remove listener once when unsubscribe is called', () => {
const store = createStore(reducers.todos);
const listenerA = expect.createSpy(() => {});
const listenerB = expect.createSpy(() => {});
Expand All @@ -217,6 +217,20 @@ describe('createStore', () => {
expect(listenerB.calls.length).toBe(1);
});

it('should only remove relevant listener when unsubscribe is called', () => {
const store = createStore(reducers.todos);
const listener = expect.createSpy(() => {});

store.subscribe(listener);
const unsubscribeSecond = store.subscribe(listener);

unsubscribeSecond();
unsubscribeSecond();

store.dispatch(unknownAction());
expect(listener.calls.length).toBe(1);
});

it('should support removing a subscription within a subscription', () => {
const store = createStore(reducers.todos);
const listenerA = expect.createSpy(() => {});
Expand Down

0 comments on commit b7031ce

Please sign in to comment.