Skip to content

Commit

Permalink
[fixed] tearing down location listeners
Browse files Browse the repository at this point in the history
when a router component was unmounted, the
locations continued listening, now they are
actually removed
  • Loading branch information
ryanflorence committed Dec 15, 2014
1 parent 56986af commit 6417285
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 0 deletions.
22 changes: 22 additions & 0 deletions modules/__tests__/Router-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -970,3 +970,25 @@ describe('Router.run', function () {
});

});

describe('unmounting', function () {
afterEach(function() {
window.location.hash = '';
});

it('removes location change listeners', function (done) {
var c = 0;
var div = document.createElement('div');
Router.run(<Route handler={Foo} path="*"/>, Router.HashLocation, function(Handler) {
c++;
expect(c).toEqual(1);
React.renderComponent(<Handler/>, div, function() {
React.unmountComponentAtNode(div);
Router.HashLocation.push('/foo');
// might be flakey? I wish I knew right now a better way to do this
setTimeout(done, 0);
});
});

});
});
20 changes: 20 additions & 0 deletions modules/locations/HashLocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,26 @@ var HashLocation = {
_isListening = true;
},

removeChangeListener: function(listener) {
for (var i = 0, l = _changeListeners.length; i < l; i ++) {
if (_changeListeners[i] === listener) {
_changeListeners.splice(i, 1);
break;
}
}

if (window.removeEventListener) {
window.removeEventListener('hashchange', onHashChange, false);
} else {
window.removeEvent('onhashchange', onHashChange);
}

if (_changeListeners.length === 0)
_isListening = false;
},



push: function (path) {
_actionType = LocationActions.PUSH;
window.location.hash = Path.encode(path);
Expand Down
20 changes: 20 additions & 0 deletions modules/locations/HistoryLocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,26 @@ var HistoryLocation = {
_isListening = true;
},

removeChangeListener: function(listener) {
for (var i = 0, l = _changeListeners.length; i < l; i ++) {
if (_changeListeners[i] === listener) {
_changeListeners.splice(i, 1);
break;
}
}

if (window.addEventListener) {
window.removeEventListener('popstate', onPopState);
} else {
window.removeEvent('popstate', onPopState);
}

if (_changeListeners.length === 0)
_isListening = false;
},



push: function (path) {
window.history.pushState({ path: path }, '', Path.encode(path));
History.length += 1;
Expand Down
2 changes: 2 additions & 0 deletions modules/locations/TestLocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ var TestLocation = {
updateHistoryLength();
},

removeChangeListener: function () {},

push: function (path) {
TestLocation.history.push(path);
updateHistoryLength();
Expand Down
8 changes: 8 additions & 0 deletions modules/utils/createRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,10 @@ function createRouter(options) {
// Bootstrap using the current path.
router.dispatch(location.getCurrentPath(), null, dispatchHandler);
}
},

teardown: function() {
location.removeChangeListener(this.changeListener);
}

},
Expand Down Expand Up @@ -439,6 +443,10 @@ function createRouter(options) {
this.setState(state);
},

componentWillUnmount: function() {
router.teardown();
},

render: function () {
return this.getRouteAtDepth(0) ? React.createElement(RouteHandler, this.props) : null;
},
Expand Down

0 comments on commit 6417285

Please sign in to comment.