Skip to content

Commit

Permalink
nullObjects [nfc]: Annotate NULL_ARRAY.
Browse files Browse the repository at this point in the history
This will help move us along toward Flow's new "Types-First" mode;
switching entirely is zulip#4907.

And chase through many Flow errors pointing out that various other
object and array types should be marked read-only too. All of them
look like they should be read-only anyway; our read-only markers
haven't generally kept up with all the places we pass around objects
and arrays expecting them not to be mutated.

After adding the annotation, I went through five iterations of

- run `yarn flow`
- resolve each error in the output (and no others) by
  command-clicking to the type that Flow says should be made
  read-only, and making it so

, stopping when Flow didn't show any more errors.
  • Loading branch information
chrisbobbe authored and gnprice committed Aug 12, 2021
1 parent 98ceb66 commit 3c274c3
Show file tree
Hide file tree
Showing 17 changed files with 62 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/api/initialDataTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export type HuddlesUnreadItem = {|
user_ids_string: string,

// Sorted.
unread_message_ids: number[],
unread_message_ids: $ReadOnlyArray<number>,
|};

/** The unreads in a 1:1 PM thread, as represented in `unread_msgs`. */
Expand All @@ -259,7 +259,7 @@ export type PmsUnreadItem = {|
sender_id: UserId,

// Sorted.
unread_message_ids: number[],
unread_message_ids: $ReadOnlyArray<number>,
|};

/** Initial data for `update_message_flags` events. */
Expand Down
8 changes: 5 additions & 3 deletions src/chat/narrowsSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { shouldBeMuted } from '../utils/message';
import { NULL_ARRAY, NULL_SUBSCRIPTION } from '../nullObjects';
import * as logging from '../utils/logging';

export const outboxMessagesForNarrow: Selector<Outbox[], Narrow> = createSelector(
export const outboxMessagesForNarrow: Selector<$ReadOnlyArray<Outbox>, Narrow> = createSelector(
(state, narrow) => narrow,
getCaughtUpForNarrow,
state => getOutbox(state),
Expand All @@ -57,8 +57,10 @@ export const outboxMessagesForNarrow: Selector<Outbox[], Narrow> = createSelecto
},
);

export const getFetchedMessageIdsForNarrow = (state: GlobalState, narrow: Narrow): number[] =>
getAllNarrows(state).get(keyFromNarrow(narrow)) || NULL_ARRAY;
export const getFetchedMessageIdsForNarrow = (
state: GlobalState,
narrow: Narrow,
): $ReadOnlyArray<number> => getAllNarrows(state).get(keyFromNarrow(narrow)) || NULL_ARRAY;

const getFetchedMessagesForNarrow: Selector<Message[], Narrow> = createSelector(
getFetchedMessageIdsForNarrow,
Expand Down
8 changes: 4 additions & 4 deletions src/message/messageActionSheet.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type TopicArgs = {
auth: Auth,
streamId: number,
topic: string,
subscriptions: Subscription[],
subscriptions: $ReadOnlyArray<Subscription>,
streams: Map<number, Stream>,
dispatch: Dispatch,
_: GetText,
Expand Down Expand Up @@ -240,7 +240,7 @@ export const constructTopicActionButtons = ({
backgroundData: $ReadOnly<{
mute: MuteState,
streams: Map<number, Stream>,
subscriptions: Subscription[],
subscriptions: $ReadOnlyArray<Subscription>,
unread: UnreadState,
ownUser: User,
...
Expand Down Expand Up @@ -380,7 +380,7 @@ export const showMessageActionSheet = ({
|},
backgroundData: $ReadOnly<{
auth: Auth,
subscriptions: Subscription[],
subscriptions: $ReadOnlyArray<Subscription>,
ownUser: User,
flags: FlagsState,
...
Expand Down Expand Up @@ -419,7 +419,7 @@ export const showTopicActionSheet = ({
auth: Auth,
mute: MuteState,
streams: Map<number, Stream>,
subscriptions: Subscription[],
subscriptions: $ReadOnlyArray<Subscription>,
unread: UnreadState,
ownUser: User,
flags: FlagsState,
Expand Down
2 changes: 1 addition & 1 deletion src/message/messageSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { NULL_ARRAY } from '../nullObjects';
* Returns something which may or may not be an array, but is at least JSONable
* and human-readable.
*/
function truncateForLogging<T: JSONable>(arr: Array<T>, len = 10): JSONable {
function truncateForLogging<T: JSONable>(arr: $ReadOnlyArray<T>, len = 10): JSONable {
if (arr.length <= 2 * len) {
return arr;
}
Expand Down
2 changes: 1 addition & 1 deletion src/nav/getInitialRouteInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { Account } from '../types';

export default (args: {|
hasAuth: boolean,
accounts: Account[],
accounts: $ReadOnlyArray<Account>,
|}): {| initialRouteName: string, initialRouteParams?: ScreenParams |} => {
const { hasAuth, accounts } = args;

Expand Down
2 changes: 1 addition & 1 deletion src/nullObjects.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { Subscription } from './types';

export const NULL_OBJECT: {||} = Object.freeze({});

export const NULL_ARRAY = Object.freeze([]);
export const NULL_ARRAY: $ReadOnlyArray<empty> = Object.freeze([]);

/*
* All the below objects are DEPRECATED; rather than using one, choose the
Expand Down
16 changes: 8 additions & 8 deletions src/reduxTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export type * from './actionTypes';
* the app.
* * `Account` for details on the properties of each account object.
*/
export type AccountsState = Account[];
export type AccountsState = $ReadOnlyArray<Account>;

export type AlertWordsState = string[];
export type AlertWordsState = $ReadOnlyArray<string>;

/**
* Info about how complete our knowledge is of the messages in some narrow.
Expand Down Expand Up @@ -174,7 +174,7 @@ export type MigrationsState = {|
version?: string,
|};

export type MuteState = MuteTuple[];
export type MuteState = $ReadOnlyArray<MuteTuple>;

/** A map from user IDs to the Unix timestamp in seconds when they were muted. */
export type MutedUsersState = Immutable.Map<UserId, number>;
Expand All @@ -199,7 +199,7 @@ export type MutedUsersState = Immutable.Map<UserId, number>;
*/
export type NarrowsState = Immutable.Map<string, number[]>;

export type OutboxState = Outbox[];
export type OutboxState = $ReadOnlyArray<Outbox>;

/**
* The `presence` subtree of our Redux state.
Expand Down Expand Up @@ -300,9 +300,9 @@ export type SettingsState = {|
doNotMarkMessagesAsRead: boolean,
|};

export type StreamsState = Stream[];
export type StreamsState = $ReadOnlyArray<Stream>;

export type SubscriptionsState = Subscription[];
export type SubscriptionsState = $ReadOnlyArray<Subscription>;

export type TopicsState = {|
[number]: Topic[],
Expand All @@ -315,7 +315,7 @@ export type TypingState = {|
|},
|};

export type UserGroupsState = UserGroup[];
export type UserGroupsState = $ReadOnlyArray<UserGroup>;

export type UserStatusState = UserStatusMapObject;

Expand All @@ -325,7 +325,7 @@ export type UserStatusState = UserStatusMapObject;
* This contains all users except deactivated users and cross-realm bots.
* For those, see RealmState.
*/
export type UsersState = User[];
export type UsersState = $ReadOnlyArray<User>;

/**
* Our complete Redux state tree.
Expand Down
4 changes: 2 additions & 2 deletions src/subscriptions/StreamListCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ type Props = $ReadOnly<{|
dispatch: Dispatch,
auth: Auth,
canCreateStreams: boolean,
streams: Stream[],
subscriptions: Subscription[],
streams: $ReadOnlyArray<Stream>,
subscriptions: $ReadOnlyArray<Subscription>,
|}>;

class StreamListCard extends PureComponent<Props> {
Expand Down
2 changes: 1 addition & 1 deletion src/topics/topicSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { getStreamsById } from '../subscriptions/subscriptionSelectors';
import { NULL_ARRAY } from '../nullObjects';
import { isStreamNarrow, streamNameOfNarrow } from '../utils/narrow';

export const getTopicsForNarrow: Selector<string[], Narrow> = createSelector(
export const getTopicsForNarrow: Selector<$ReadOnlyArray<string>, Narrow> = createSelector(
(state, narrow) => narrow,
state => getTopics(state),
state => getStreams(state),
Expand Down
2 changes: 1 addition & 1 deletion src/typing/typingSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const getCurrentTypingUsers: Selector<$ReadOnlyArray<UserOrBot>, Narrow>
(state, narrow) => narrow,
state => getTyping(state),
state => getAllUsersById(state),
(narrow, typing, allUsersById): UserOrBot[] => {
(narrow, typing, allUsersById): $ReadOnlyArray<UserOrBot> => {
if (!isPmNarrow(narrow)) {
return NULL_ARRAY;
}
Expand Down
21 changes: 14 additions & 7 deletions src/unread/unreadHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
import type { HuddlesUnreadItem, PmsUnreadItem, UserId } from '../types';
import { addItemsToArray, removeItemsFromArray, filterArray } from '../utils/immutability';

type SomeUnreadItem = { unread_message_ids: number[], ... };
type SomeUnreadItem = { unread_message_ids: $ReadOnlyArray<number>, ... };

export function removeItemsDeeply<T: SomeUnreadItem>(objArray: T[], messageIds: number[]): T[] {
export function removeItemsDeeply<T: SomeUnreadItem>(
objArray: $ReadOnlyArray<T>,
messageIds: number[],
): $ReadOnlyArray<T> {
let changed = false;
const objWithAddedUnreadIds = objArray.map(obj => {
const filteredIds = removeItemsFromArray(obj.unread_message_ids, messageIds);
Expand All @@ -29,7 +32,11 @@ export function removeItemsDeeply<T: SomeUnreadItem>(objArray: T[], messageIds:
);
}

function addItemsDeeply<T: SomeUnreadItem>(input: T[], itemsToAdd: number[], index: number): T[] {
function addItemsDeeply<T: SomeUnreadItem>(
input: $ReadOnlyArray<T>,
itemsToAdd: number[],
index: number,
): $ReadOnlyArray<T> {
const item = input[index];

const unreadMessageIds = addItemsToArray(item.unread_message_ids, itemsToAdd);
Expand All @@ -48,10 +55,10 @@ function addItemsDeeply<T: SomeUnreadItem>(input: T[], itemsToAdd: number[], ind
}

export const addItemsToPmArray = (
input: PmsUnreadItem[],
input: $ReadOnlyArray<PmsUnreadItem>,
itemsToAdd: number[],
senderId: UserId,
): PmsUnreadItem[] => {
): $ReadOnlyArray<PmsUnreadItem> => {
const index = input.findIndex(sender => sender.sender_id === senderId);

if (index !== -1) {
Expand All @@ -68,10 +75,10 @@ export const addItemsToPmArray = (
};

export const addItemsToHuddleArray = (
input: HuddlesUnreadItem[],
input: $ReadOnlyArray<HuddlesUnreadItem>,
itemsToAdd: number[],
userIds: string,
): HuddlesUnreadItem[] => {
): $ReadOnlyArray<HuddlesUnreadItem> => {
const index = input.findIndex(recipients => recipients.user_ids_string === userIds);

if (index !== -1) {
Expand Down
6 changes: 3 additions & 3 deletions src/unread/unreadModelTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export type UnreadStreamsState =
*
* Part of `UnreadState`; see there for more.
*/
export type UnreadHuddlesState = HuddlesUnreadItem[];
export type UnreadHuddlesState = $ReadOnlyArray<HuddlesUnreadItem>;

/**
* A summary of (almost) all unread 1:1 PMs.
Expand All @@ -41,7 +41,7 @@ export type UnreadHuddlesState = HuddlesUnreadItem[];
* Part of `UnreadState`; see there for more.
* See in particular `UnreadHuddlesState` for group PMs.
*/
export type UnreadPmsState = PmsUnreadItem[];
export type UnreadPmsState = $ReadOnlyArray<PmsUnreadItem>;

/**
* A summary of (almost) all unread messages with @-mentions.
Expand All @@ -54,7 +54,7 @@ export type UnreadPmsState = PmsUnreadItem[];
*
* Part of `UnreadState`; see there for more.
*/
export type UnreadMentionsState = number[];
export type UnreadMentionsState = $ReadOnlyArray<number>;

/**
* A summary of (almost) all unread messages, even those we don't have.
Expand Down
2 changes: 1 addition & 1 deletion src/users/userHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export const getAutocompleteSuggestion = (
};

export const getAutocompleteUserGroupSuggestions = (
userGroups: UserGroup[],
userGroups: $ReadOnlyArray<UserGroup>,
filter: string = '',
): UserGroup[] =>
userGroups.filter(
Expand Down
12 changes: 9 additions & 3 deletions src/utils/immutability.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
/* @flow strict-local */

export const removeItemsFromArray = (input: number[], itemsToRemove: number[]): number[] => {
export const removeItemsFromArray = (
input: $ReadOnlyArray<number>,
itemsToRemove: number[],
): $ReadOnlyArray<number> => {
const output = input.filter((item: number) => !itemsToRemove.includes(item));
return input.length === output.length ? input : output;
};

export function addItemsToArray<T>(input: T[], itemsToAdd: T[]): T[] {
export function addItemsToArray<T>(input: $ReadOnlyArray<T>, itemsToAdd: T[]): $ReadOnlyArray<T> {
const newItems = itemsToAdd.filter(item => !input.includes(item));
return newItems.length > 0 ? [...input, ...itemsToAdd] : input;
}

export function filterArray<T>(input: T[], predicate: T => boolean): T[] {
export function filterArray<T>(
input: $ReadOnlyArray<T>,
predicate: T => boolean,
): $ReadOnlyArray<T> {
const filteredList = input.filter(predicate);
return filteredList.length === input.length ? input : filteredList;
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const isTopicMuted = (stream: string, topic: string, mute: MuteState = []
export const shouldBeMuted = (
message: Message | Outbox,
narrow: Narrow,
subscriptions: Subscription[] = [],
subscriptions: $ReadOnlyArray<Subscription> = [],
mutes: MuteState = [],
): boolean => {
if (message.type === 'private') {
Expand Down
2 changes: 1 addition & 1 deletion src/webview/MessageList.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export type BackgroundData = $ReadOnly<{|
ownUser: User,
streams: Map<number, Stream>,
streamsByName: Map<string, Stream>,
subscriptions: Subscription[],
subscriptions: $ReadOnlyArray<Subscription>,
unread: UnreadState,
theme: ThemeName,
twentyFourHourTime: boolean,
Expand Down
9 changes: 7 additions & 2 deletions src/webview/html/processAlertWords.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function escape_user_regex(value) {

/* Helper borrowed near-verbatim from webapp; see comment above. */
// prettier-ignore
const process_message = function (words: string[],
const process_message = function (words: $ReadOnlyArray<string>,
message: {| alerted: boolean, content: string |}) {
// Parsing for alert words is expensive, so we rely on the host
// to tell us there any alert words to even look for.
Expand Down Expand Up @@ -58,7 +58,12 @@ const process_message = function (words: string[],
};

/** Mark any alert words in `content` with an appropriate span. */
export default (content: string, id: number, alertWords: string[], flags: FlagsState): string => {
export default (
content: string,
id: number,
alertWords: $ReadOnlyArray<string>,
flags: FlagsState,
): string => {
// This is kind of funny style, but lets us borrow the webapp's code near
// verbatim, inside `process_message`.
let message = { content, alerted: flags.has_alert_word[id] };
Expand Down

0 comments on commit 3c274c3

Please sign in to comment.