Merge pull request #31 from MicheleBertoli/unmounted

does not call the callback on unmounted components
MicheleBertoli · Dec 23, 2015 · bd29013 · bd29013
2 parents 629b6e6 + 2f6c7a0
commit bd29013
Show file tree

Hide file tree

Showing 8 changed files with 101 additions and 20 deletions.
diff --git a/dist/components/__tests__/gmaps-test.js b/dist/components/__tests__/gmaps-test.js
@@ -152,4 +152,30 @@ describe('Gmaps', function () {
       expect(parent.refs.gmaps.getMap().setOptions).toBeCalled();
     });
   });
+
+  describe('unmounted', function () {
+
+    beforeEach(function () {
+      GoogleMaps.fireCallbacks = jest.genMockFunction();
+    });
+
+    it('does not fire the callback (unloaded)', function () {
+      var gmaps = TestUtils.renderIntoDocument(React.createElement(Gmaps, null));
+      ReactDOM.unmountComponentAtNode(ReactDOM.findDOMNode(gmaps).parentNode);
+      expect(GoogleMaps.fireCallbacks).not.toBeCalled();
+    });
+
+    it('does not fire the callback (loaded)', function () {
+      window.google = {
+        maps: {
+          event: {
+            removeListener: jest.genMockFunction()
+          }
+        }
+      };
+      var gmaps = TestUtils.renderIntoDocument(React.createElement(Gmaps, null));
+      ReactDOM.unmountComponentAtNode(ReactDOM.findDOMNode(gmaps).parentNode);
+      expect(GoogleMaps.fireCallbacks).not.toBeCalled();
+    });
+  });
 });
diff --git a/dist/components/gmaps.js b/dist/components/gmaps.js
@@ -50,10 +50,13 @@ var Gmaps = _react2['default'].createClass({
   },
 
   componentDidMount: function componentDidMount() {
-    _utilsGoogleMaps2['default'].load(this.props.params, this.mapsCallback);
+    this.setState({
+      callbackIndex: _utilsGoogleMaps2['default'].load(this.props.params, this.mapsCallback)
+    });
   },
 
   componentWillUnmount: function componentWillUnmount() {
+    _utilsGoogleMaps2['default'].removeCallback(this.state.callbackIndex);
     this.removeListeners();
   },
 
@@ -83,7 +86,7 @@ var Gmaps = _react2['default'].createClass({
       isMapCreated: true
     });
     if (this.props.onMapCreated) {
-      this.props.onMapCreated(this.map, google.maps);
+      this.props.onMapCreated(this.map);
     }
   },
 

diff --git a/dist/mixins/listener.js b/dist/mixins/listener.js
@@ -19,9 +19,11 @@ var Listener = {
   },
 
   removeListeners: function removeListeners() {
-    this.listeners.forEach(function (listener) {
-      google.maps.event.removeListener(listener);
-    });
+    if (window.google) {
+      this.listeners.forEach(function (listener) {
+        google.maps.event.removeListener(listener);
+      });
+    }
   }
 
 };

diff --git a/dist/utils/google-maps.js b/dist/utils/google-maps.js
@@ -17,15 +17,16 @@ exports['default'] = {
   appended: false,
 
   load: function load(params, callback) {
-    if (!window.google) {
-      this.callbacks.push(callback);
+    var index = this.callbacks.push(callback);
+    if (window.google) {
+      setTimeout(this.fireCallbacks.bind(this));
+    } else {
       if (!this.appended) {
         window.mapsCallback = this.mapsCallback.bind(this);
         this.appendScript(params);
       }
-    } else {
-      setTimeout(callback);
     }
+    return index;
   },
 
   getSrc: function getSrc(params) {
@@ -44,11 +45,19 @@ exports['default'] = {
   },
 
   mapsCallback: function mapsCallback() {
-    window.mapsCallback = undefined;;
+    window.mapsCallback = undefined;
+    this.fireCallbacks();
+  },
+
+  fireCallbacks: function fireCallbacks() {
     this.callbacks.forEach(function (callback) {
       return callback();
     });
     this.callbacks = [];
+  },
+
+  removeCallback: function removeCallback(index) {
+    this.callbacks.splice(index - 1, 1);
   }
 
 };

diff --git a/src/components/__tests__/gmaps-test.js b/src/components/__tests__/gmaps-test.js
@@ -148,4 +148,31 @@ describe('Gmaps', () => {
 
   });
 
+  describe('unmounted', () => {
+
+    beforeEach(() => {
+      GoogleMaps.fireCallbacks = jest.genMockFunction();
+    });
+
+    it('does not fire the callback (unloaded)', () => {
+      const gmaps = TestUtils.renderIntoDocument(<Gmaps />);
+      ReactDOM.unmountComponentAtNode(ReactDOM.findDOMNode(gmaps).parentNode);
+      expect(GoogleMaps.fireCallbacks).not.toBeCalled();
+    });
+
+    it('does not fire the callback (loaded)', () => {
+      window.google = {
+        maps: {
+          event: {
+            removeListener: jest.genMockFunction()
+          }
+        }
+      };
+      const gmaps = TestUtils.renderIntoDocument(<Gmaps />);
+      ReactDOM.unmountComponentAtNode(ReactDOM.findDOMNode(gmaps).parentNode);
+      expect(GoogleMaps.fireCallbacks).not.toBeCalled();
+    });
+
+  });
+
 });
diff --git a/src/components/gmaps.js b/src/components/gmaps.js
@@ -19,10 +19,13 @@ const Gmaps = React.createClass({
   },
 
   componentDidMount() {
-    GoogleMaps.load(this.props.params, this.mapsCallback);
+    this.setState({
+      callbackIndex: GoogleMaps.load(this.props.params, this.mapsCallback)
+    });
   },
 
   componentWillUnmount() {
+    GoogleMaps.removeCallback(this.state.callbackIndex);
     this.removeListeners();
   },
 
@@ -54,7 +57,7 @@ const Gmaps = React.createClass({
       isMapCreated: true
     });
     if (this.props.onMapCreated) {
-      this.props.onMapCreated(this.map, google.maps);
+      this.props.onMapCreated(this.map);
     }
   },
 

diff --git a/src/mixins/listener.js b/src/mixins/listener.js
@@ -14,9 +14,11 @@ const Listener = {
   },
 
   removeListeners() {
-    this.listeners.forEach((listener) => {
-      google.maps.event.removeListener(listener);
-    });
+    if (window.google) {
+      this.listeners.forEach((listener) => {
+        google.maps.event.removeListener(listener);
+      });
+    }
   }
 
 };

diff --git a/src/utils/google-maps.js b/src/utils/google-maps.js
@@ -7,15 +7,16 @@ export default {
   appended: false,
 
   load(params, callback) {
-    if (!window.google) {
-      this.callbacks.push(callback);
+    const index = this.callbacks.push(callback);
+    if (window.google) {
+      setTimeout(this.fireCallbacks.bind(this));
+    } else {
       if (!this.appended) {
         window.mapsCallback = this.mapsCallback.bind(this);
         this.appendScript(params);
       }
-    } else {
-      setTimeout(callback);
     }
+    return index;
   },
 
   getSrc(params) {
@@ -34,9 +35,17 @@ export default {
   },
 
   mapsCallback() {
-    window.mapsCallback = undefined;;
+    window.mapsCallback = undefined;
+    this.fireCallbacks();
+  },
+
+  fireCallbacks() {
     this.callbacks.forEach(callback => callback());
     this.callbacks = [];
+  },
+
+  removeCallback(index) {
+    this.callbacks.splice(index - 1, 1);
   }
 
 };