Skip to content

Commit 6417285

Browse files
committed
[fixed] tearing down location listeners
when a router component was unmounted, the locations continued listening, now they are actually removed
1 parent 56986af commit 6417285

File tree

5 files changed

+72
-0
lines changed

5 files changed

+72
-0
lines changed

modules/__tests__/Router-test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,3 +970,25 @@ describe('Router.run', function () {
970970
});
971971

972972
});
973+
974+
describe('unmounting', function () {
975+
afterEach(function() {
976+
window.location.hash = '';
977+
});
978+
979+
it('removes location change listeners', function (done) {
980+
var c = 0;
981+
var div = document.createElement('div');
982+
Router.run(<Route handler={Foo} path="*"/>, Router.HashLocation, function(Handler) {
983+
c++;
984+
expect(c).toEqual(1);
985+
React.renderComponent(<Handler/>, div, function() {
986+
React.unmountComponentAtNode(div);
987+
Router.HashLocation.push('/foo');
988+
// might be flakey? I wish I knew right now a better way to do this
989+
setTimeout(done, 0);
990+
});
991+
});
992+
993+
});
994+
});

modules/locations/HashLocation.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,26 @@ var HashLocation = {
7979
_isListening = true;
8080
},
8181

82+
removeChangeListener: function(listener) {
83+
for (var i = 0, l = _changeListeners.length; i < l; i ++) {
84+
if (_changeListeners[i] === listener) {
85+
_changeListeners.splice(i, 1);
86+
break;
87+
}
88+
}
89+
90+
if (window.removeEventListener) {
91+
window.removeEventListener('hashchange', onHashChange, false);
92+
} else {
93+
window.removeEvent('onhashchange', onHashChange);
94+
}
95+
96+
if (_changeListeners.length === 0)
97+
_isListening = false;
98+
},
99+
100+
101+
82102
push: function (path) {
83103
_actionType = LocationActions.PUSH;
84104
window.location.hash = Path.encode(path);

modules/locations/HistoryLocation.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,26 @@ var HistoryLocation = {
5050
_isListening = true;
5151
},
5252

53+
removeChangeListener: function(listener) {
54+
for (var i = 0, l = _changeListeners.length; i < l; i ++) {
55+
if (_changeListeners[i] === listener) {
56+
_changeListeners.splice(i, 1);
57+
break;
58+
}
59+
}
60+
61+
if (window.addEventListener) {
62+
window.removeEventListener('popstate', onPopState);
63+
} else {
64+
window.removeEvent('popstate', onPopState);
65+
}
66+
67+
if (_changeListeners.length === 0)
68+
_isListening = false;
69+
},
70+
71+
72+
5373
push: function (path) {
5474
window.history.pushState({ path: path }, '', Path.encode(path));
5575
History.length += 1;

modules/locations/TestLocation.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ var TestLocation = {
2828
updateHistoryLength();
2929
},
3030

31+
removeChangeListener: function () {},
32+
3133
push: function (path) {
3234
TestLocation.history.push(path);
3335
updateHistoryLength();

modules/utils/createRouter.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,10 @@ function createRouter(options) {
404404
// Bootstrap using the current path.
405405
router.dispatch(location.getCurrentPath(), null, dispatchHandler);
406406
}
407+
},
408+
409+
teardown: function() {
410+
location.removeChangeListener(this.changeListener);
407411
}
408412

409413
},
@@ -439,6 +443,10 @@ function createRouter(options) {
439443
this.setState(state);
440444
},
441445

446+
componentWillUnmount: function() {
447+
router.teardown();
448+
},
449+
442450
render: function () {
443451
return this.getRouteAtDepth(0) ? React.createElement(RouteHandler, this.props) : null;
444452
},

0 commit comments

Comments
 (0)