Implement bulk invite rejections #592
Conversation
| @@ -101,6 +108,7 @@ module.exports = React.createClass({ | |||
| middleOpacity: 1.0, | |||
| }); | |||
| dis.unregister(this.dispatcherRef); | |||
| MatrixClientPeg.get().removeListener("RoomMember.membership", this._onInviteStateChange); | |||
There was a problem hiding this comment.
we might have been logged out by the time we get here; needs a null check
There was a problem hiding this comment.
You mean MatrixClientPeg.get() is nullable?!
There was a problem hiding this comment.
$ grep -rn "MatrixClientPeg\.get()\." matrix-react-sdk/src/ | wc -l
264
There are a lot of assumptions in other UI components that assume MatrixClientPeg is not null. These are generally safe assumptions because the UI component in question isn't shown when the user is not logged in: as is the case here. I would rather not increase the SNR any more than I need to. WDYT?
There was a problem hiding this comment.
yup, MatrixClientPeg.get() will return null after you log out.
In general you are right, but this is in an componentWillUnmount method which will be called when the user logs out. You should find that any other references to MatrixClientPeg.get in componentWillUnmount have null guards - any which don't are also bugs.
(We're actually in the process of switching to passing the MatrixClient via the react context rather than using the MatrixClientPeg singleton, which means that components can rely on the MatrixClient not changing under them. But we're already using MatrixClientPeg in this component and I'd rather not mix usage in a single component.)
| this.setState({ | ||
| rejectingInvites: false | ||
| }); | ||
| }); |
There was a problem hiding this comment.
still needs a .done() to catch exceptions from the setState?
There was a problem hiding this comment.
Ugh, yes you're right.
/me pines for bluebird
|
PTAL @richvdh |
No description provided.