-
-
Notifications
You must be signed in to change notification settings - Fork 827
Conform more of the codebase to strictNullChecks
#10738
Conversation
permalinkCreator={this.props.permalinkCreator} | ||
/> | ||
); | ||
if (roomId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (roomId) { | |
if (!!roomId && !!this.props.permalinkCreator) { |
(to fix implicit boolean cast, and type failure on permalinkCreator)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I specifically didn't touch permalinkCreator as there are multiple related type fails around the code and it may be intentional that it can be passed as undefined
@@ -1087,7 +1087,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> { | |||
case "picture_snapshot": | |||
ContentMessages.sharedInstance().sendContentListToRoom( | |||
[payload.file], | |||
this.state.room.roomId, | |||
this.getRoomId(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is still failing. Likewise various similar references below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intentional, consolidates everything to use this single method which in future will be made non-optional and to throw an error if no roomId so it is picked up by the error boundary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was deferred to a later task, this just consolidates the handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks plausible, but the merge makes it difficult to review changes since the last review, and I don't want to have to go through the whole lot all over again. Could you keep these PRs smaller please? It's difficult for reviewers to juggle this much state.
Requires matrix-org/matrix-react-sdk-module-api#15Requires matrix-org/matrix-js-sdk#3330Fixes element-hq/element-web#24752
This change is marked as an internal change (Task), so will not be included in the changelog.