Merge pull request #84 from matrix-org/rav/fix_refs_npes

Fix some races due to promises completing after we've switched rooms
This commit is contained in:
Richard van der Hoff 2016-01-11 15:25:17 +00:00
commit e7740cbc8b
2 changed files with 33 additions and 6 deletions

View file

@ -118,6 +118,12 @@ module.exports = React.createClass({
},
componentWillUnmount: function() {
// set a boolean to say we've been unmounted, which any pending
// promises can use to throw away their results.
//
// (We could use isMounted, but facebook have deprecated that.)
this.unmounted = true;
if (this.refs.messagePanel) {
// disconnect the D&D event listeners from the message panel. This
// is really just for hygiene - the messagePanel is going to be
@ -214,7 +220,7 @@ module.exports = React.createClass({
},*/
onRoomTimeline: function(ev, room, toStartOfTimeline) {
if (!this.isMounted()) return;
if (this.unmounted) return;
// ignore anything that comes in whilst paginating: we get one
// event for each new matrix event so this would cause a huge
@ -371,11 +377,14 @@ module.exports = React.createClass({
_paginateCompleted: function() {
debuglog("paginate complete");
this.setState({
room: MatrixClientPeg.get().getRoom(this.props.roomId)
});
// we might have switched rooms since the paginate started - just bin
// the results if so.
if (this.unmounted) return;
this.setState({paginating: false});
this.setState({
room: MatrixClientPeg.get().getRoom(this.props.roomId),
paginating: false,
});
},
onSearchResultsFillRequest: function(backwards) {
@ -559,7 +568,7 @@ module.exports = React.createClass({
return searchPromise.then(function(results) {
debuglog("search complete");
if (!self.state.searching || self.searchId != localSearchId) {
if (self.unmounted || !self.state.searching || self.searchId != localSearchId) {
console.error("Discarding stale search results");
return;
}

View file

@ -112,6 +112,14 @@ module.exports = React.createClass({
this.checkFillState();
},
componentWillUnmount: function() {
// set a boolean to say we've been unmounted, which any pending
// promises can use to throw away their results.
//
// (We could use isMounted(), but facebook have deprecated that.)
this.unmounted = true;
},
onScroll: function(ev) {
var sn = this._getScrollNode();
debuglog("Scroll event: offset now:", sn.scrollTop, "recentEventScroll:", this.recentEventScroll);
@ -158,6 +166,10 @@ module.exports = React.createClass({
// check the scroll state and send out backfill requests if necessary.
checkFillState: function() {
if (this.unmounted) {
return;
}
var sn = this._getScrollNode();
// if there is less than a screenful of messages above or below the
@ -346,6 +358,12 @@ module.exports = React.createClass({
* message panel.
*/
_getScrollNode: function() {
if (this.unmounted) {
// this shouldn't happen, but when it does, turn the NPE into
// something more meaningful.
throw new Error("ScrollPanel._getScrollNode called when unmounted");
}
var panel = ReactDOM.findDOMNode(this.refs.geminiPanel);
// If the gemini scrollbar is doing its thing, this will be a div within