From d5a522fdfe7e79b88d7f2ecc54ee4a81877a6c4f Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 7 Jun 2016 18:22:01 +0100 Subject: [PATCH] Second attempt at fixing the Velocity memory leak 1) Correct fix for Velociraptor (we need to find the DOM node and pass that in) 2) Do the same leak fix for the read marker 3) Update the dependency to our fork which is fixed to make the call we do to release memory actually work. 4) Remove the velocity-ui-pack dependency which is unnecessary because velocity-ui is included in the velocity package --- package.json | 3 +-- src/Velociraptor.js | 3 ++- src/components/structures/MessagePanel.js | 17 ++++++++++++++--- src/components/views/login/RegistrationForm.js | 2 +- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 7dce6b3d79..6bea06f075 100644 --- a/package.json +++ b/package.json @@ -37,8 +37,7 @@ "react-dom": "^15.0.1", "react-gemini-scrollbar": "matrix-org/react-gemini-scrollbar#c3d942e", "sanitize-html": "^1.11.1", - "velocity-animate": "^1.2.3", - "velocity-ui-pack": "^1.2.2" + "velocity-animate": "vector-im/velocity#a70ebc7" }, "//babelversion": [ "brief experiments with babel6 seems to show that it generates source ", diff --git a/src/Velociraptor.js b/src/Velociraptor.js index 0abf34b230..2a8453c7ea 100644 --- a/src/Velociraptor.js +++ b/src/Velociraptor.js @@ -117,7 +117,8 @@ module.exports = React.createClass({ // and the FAQ entry, "Preventing memory leaks when // creating/destroying large numbers of elements" // (https://github.com/julianshapiro/velocity/issues/47) - Velocity.Utilities.removeData(this.nodes[k]); + var domNode = ReactDom.findDOMNode(this.nodes[k]); + Velocity.Utilities.removeData(domNode); } this.nodes[k] = node; }, diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 16b4892bc0..af047bd190 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -86,6 +86,10 @@ module.exports = React.createClass({ // to manage its animations this._readReceiptMap = {}; + // Remember the read marker ghost node so we can do the cleanup that + // Velocity requires + this.readMarkerGhostNode = null; + this._isMounted = true; }, @@ -422,9 +426,16 @@ module.exports = React.createClass({ }, _startAnimation: function(ghostNode) { - Velocity(ghostNode, {opacity: '0', width: '10%'}, - {duration: 400, easing: 'easeInSine', - delay: 1000}); + if (this.readMarkerGhostNode) { + Velocity.Utilities.removeData(this.readMarkerGhostNode); + } + this.readMarkerGhostNode = ghostNode; + + if (ghostNode) { + Velocity(ghostNode, {opacity: '0', width: '10%'}, + {duration: 400, easing: 'easeInSine', + delay: 1000}); + } }, _getReadMarkerGhostTile: function() { diff --git a/src/components/views/login/RegistrationForm.js b/src/components/views/login/RegistrationForm.js index 83bd1ab17c..ecc02c40d2 100644 --- a/src/components/views/login/RegistrationForm.js +++ b/src/components/views/login/RegistrationForm.js @@ -18,7 +18,7 @@ limitations under the License. var React = require('react'); var Velocity = require('velocity-animate'); -require('velocity-ui-pack'); +require('velocity-animate/velocity.ui'); var sdk = require('../../../index'); var Email = require('../../../email'); var Modal = require("../../../Modal");