From 641728596d64d55ea97f32067c6f3df5b5c177b3 Mon Sep 17 00:00:00 2001 From: Ryan Florence Date: Mon, 15 Dec 2014 14:39:53 -0700 Subject: [PATCH] [fixed] tearing down location listeners when a router component was unmounted, the locations continued listening, now they are actually removed --- modules/__tests__/Router-test.js | 22 ++++++++++++++++++++++ modules/locations/HashLocation.js | 20 ++++++++++++++++++++ modules/locations/HistoryLocation.js | 20 ++++++++++++++++++++ modules/locations/TestLocation.js | 2 ++ modules/utils/createRouter.js | 8 ++++++++ 5 files changed, 72 insertions(+) diff --git a/modules/__tests__/Router-test.js b/modules/__tests__/Router-test.js index c8229773cf..7b7b286547 100644 --- a/modules/__tests__/Router-test.js +++ b/modules/__tests__/Router-test.js @@ -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(, Router.HashLocation, function(Handler) { + c++; + expect(c).toEqual(1); + React.renderComponent(, 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); + }); + }); + + }); +}); diff --git a/modules/locations/HashLocation.js b/modules/locations/HashLocation.js index e23cee039d..f7e1540757 100644 --- a/modules/locations/HashLocation.js +++ b/modules/locations/HashLocation.js @@ -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); diff --git a/modules/locations/HistoryLocation.js b/modules/locations/HistoryLocation.js index 7d0cf3d31b..cbb1d88a0a 100644 --- a/modules/locations/HistoryLocation.js +++ b/modules/locations/HistoryLocation.js @@ -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; diff --git a/modules/locations/TestLocation.js b/modules/locations/TestLocation.js index 149c22ad08..036516765d 100644 --- a/modules/locations/TestLocation.js +++ b/modules/locations/TestLocation.js @@ -28,6 +28,8 @@ var TestLocation = { updateHistoryLength(); }, + removeChangeListener: function () {}, + push: function (path) { TestLocation.history.push(path); updateHistoryLength(); diff --git a/modules/utils/createRouter.js b/modules/utils/createRouter.js index 898eeb7f2d..7dfd191482 100644 --- a/modules/utils/createRouter.js +++ b/modules/utils/createRouter.js @@ -404,6 +404,10 @@ function createRouter(options) { // Bootstrap using the current path. router.dispatch(location.getCurrentPath(), null, dispatchHandler); } + }, + + teardown: function() { + location.removeChangeListener(this.changeListener); } }, @@ -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; },