Skip to content

Commit

Permalink
connect [nfc]: Work around Flow bug by making types more explicit.
Browse files Browse the repository at this point in the history
Flow was determining that these components produced with `connect`
had type `ComponentType<empty>`, which you can see by hovering over
them in the editor wherever they're imported and used.  That means
that absolutely any set of props was acceptable to pass to them,
which is unsound.  This affected `connect` calls with a two-argument
`mapStateToProps`.

This is certainly a bug in Flow, because it's possible to work
around it in a way that doesn't give Flow any information it didn't
already have:

    const X = connect(/* ... */); // the exact same expression as before

    // $FlowFixMe
    const y = () => <X />;

    export default X;

This causes, for example, `AccountDetails` to have the beautifully
specific type `ComponentType<{| +user: User |}>`, instead of the
unsound `ComponentType<empty>`.

Fortunately, it's also possible to work around in this other way,
which adds almost no net boilerplate.  Do that.

Change prepared mechanically, like so:

  $ perl -i -0pe '
      s/connect(.*?): SelectorProps/connect<SelectorProps, _, _>$1/g
    ' src/**/*.js

Fixes: #3768
Debugged-by: Chris Bobbe <cbobbe@zulipchat.com>
Debugged-by: Ray Kraesig <rkraesig@zulipchat.com>
  • Loading branch information
gnprice authored and rk-for-zulip committed Jan 30, 2020
1 parent 00d64e4 commit fcde700
Show file tree
Hide file tree
Showing 12 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/account-info/AccountDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class AccountDetails extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
realm: getCurrentRealm(state),
userStatusText: getUserStatusTextForUser(state, props.user.user_id),
}))(AccountDetails);
2 changes: 1 addition & 1 deletion src/chat/Chat.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ class Chat extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
canSend: canSendToNarrow(props.narrow) && !getShowMessagePlaceholders(props.narrow)(state),
}))(Chat);
2 changes: 1 addition & 1 deletion src/chat/UnreadNotice.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,6 @@ class UnreadNotice extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
unreadCount: getUnreadCountForNarrow(state, props.narrow),
}))(UnreadNotice);
2 changes: 1 addition & 1 deletion src/compose/ComposeBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ class ComposeBox extends PureComponent<Props, State> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
auth: getAuth(state),
ownEmail: getOwnEmail(state),
usersByEmail: getActiveUsersByEmail(state),
Expand Down
2 changes: 1 addition & 1 deletion src/emoji/Emoji.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ class Emoji extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
imageEmoji: getAllImageEmojiByName(state)[props.name],
}))(Emoji);
2 changes: 1 addition & 1 deletion src/message/NoMessages.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class NoMessages extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
showMessagePlaceholders: getShowMessagePlaceholders(props.narrow)(state),
noMessages: getIfNoMessages(props.narrow)(state),
}))(NoMessages);
2 changes: 1 addition & 1 deletion src/nav/ChatNavBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@ class ChatNavBar extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
backgroundColor: getTitleBackgroundColor(props.narrow)(state),
}))(ChatNavBar);
2 changes: 1 addition & 1 deletion src/reactions/MessageReactionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class MessageReactionList extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
message: state.messages[props.navigation.state.params.messageId],
ownUserId: getOwnUser(state).user_id,
allUsersById: getAllUsersById(state),
Expand Down
2 changes: 1 addition & 1 deletion src/start/RealmScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,6 @@ class RealmScreen extends PureComponent<Props, State> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
initialRealm: props.navigation.state.params?.realm ?? '',
}))(RealmScreen);
2 changes: 1 addition & 1 deletion src/title-buttons/InfoNavButtonGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ class InfoNavButtonGroup extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
recipients: getRecipientsInGroupNarrow(state, props.narrow),
}))(InfoNavButtonGroup);
2 changes: 1 addition & 1 deletion src/title/TitleStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,6 @@ class TitleStream extends PureComponent<Props> {
}
}

export default connect((state, props): SelectorProps => ({
export default connect<SelectorProps, _, _>((state, props) => ({
stream: getStreamInNarrow(props.narrow)(state),
}))(TitleStream);
2 changes: 1 addition & 1 deletion src/webview/MessageList.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ type OuterProps = {|
typingUsers?: User[],
|};

export default connect((state, props: OuterProps): SelectorProps => {
export default connect<SelectorProps, _, _>((state, props: OuterProps) => {
// TODO Ideally this ought to be a caching selector that doesn't change
// when the inputs don't. Doesn't matter in a practical way here, because
// we have a `shouldComponentUpdate` that doesn't look at this prop... but
Expand Down

0 comments on commit fcde700

Please sign in to comment.