Page MenuHomePhabricator
Differential D9849

[Flow202][keyserver][skip-ci] [1/x] Fix Flow issues with conditional changeRole passed to promiseAll
ClosedPublic
Actions

Authored by ashoat on Nov 13 2023, 2:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 17, 11:39 PM
Unknown Object (File)
Mon, Mar 17, 11:39 PM
Unknown Object (File)
Mon, Mar 17, 11:38 PM
Unknown Object (File)
Mon, Mar 17, 11:31 PM
Unknown Object (File)
Sat, Mar 8, 4:40 AM
Unknown Object (File)
Feb 24 2025, 2:40 AM
Unknown Object (File)
Feb 19 2025, 6:09 AM
Unknown Object (File)
Feb 18 2025, 7:31 AM
View All 99 Files
Subscribers
None

Details

Reviewers
tomek
rohan
Commits
rCOMMaf7bbe7815bc: [Flow202][keyserver][skip-ci] [1/x] Fix Flow issues with conditional changeRole…
Summary

The new Flow is much stricter about promiseAll. It's relaxed a bit in Flow 0.214, but for now we have to comply with the additional strictness.

I separated out this diff because it required some minimal changes to thread-permission-updates.js in order to rename and export a type. This way the context for those changes is more clear.

NOTE: CI will fail on this diff. I considered the possibility of fixing Flow errors BEFORE upgrading Flow, but it wasn't possible... in some cases, the fixes to support the new version of Flow caused errors in the old version. I could have hidden these type errors with $FlowFixMe lines and then later revert those, but that seemed like too much busy work.

Depends on D9786

Test Plan

Confirm the Flow errors go away

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.Nov 13 2023, 2:42 PM
ashoat added a child revision: D9850: [Flow202][keyserver][skip-ci] [2/x] Address type error in UpdateInfosRawData.
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 13 2023, 2:53 PM
Harbormaster failed remote builds in B24065: Diff 33163!
ashoat requested review of this revision.Nov 14 2023, 6:11 AM
ashoat added a reviewer: rohan.
rohan accepted this revision.Nov 14 2023, 7:18 AM
This revision is now accepted and ready to land.Nov 14 2023, 7:18 AM
tomek accepted this revision.Nov 14 2023, 7:31 AM
ashoat retitled this revision from [Flow202][keyserver] [1/x] Fix Flow issues with conditional changeRole passed to promiseAll to [Flow202][keyserver][skip-ci] [1/x] Fix Flow issues with conditional changeRole passed to promiseAll.Nov 19 2023, 5:05 PM
ashoat edited parent revisions, added: D9984: [Flow202][web][skip-ci] [21/x] Avoid localforage.getItem with unclear type param for web encryption key; removed: D9786: [Flow202][web][skip-ci] [20/x] Fix Flow issues in ComposedMessage.Nov 26 2023, 11:21 PM
ashoat edited parent revisions, added: D10063: [Flow202][web][skip-ci] [22/x] Add updatesCurrentAsOf to PersistedKeyserverInfo; removed: D9984: [Flow202][web][skip-ci] [21/x] Avoid localforage.getItem with unclear type param for web encryption key.Nov 27 2023, 2:10 PM
This revision was landed with ongoing or failed builds.Nov 27 2023, 3:26 PM
Closed by commit rCOMMaf7bbe7815bc: [Flow202][keyserver][skip-ci] [1/x] Fix Flow issues with conditional changeRole… (authored by ashoat). · Explain Why
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMMaf7bbe7815bc: [Flow202][keyserver][skip-ci] [1/x] Fix Flow issues with conditional changeRole….

Revision Contents
Changeset List

PathSize
keyserver/
src/
creators/
17 lines
updaters/
10 lines

Diff 33163

View Options

keyserver/src/creators/thread-creator.js

  • This file was added.
// @flow
import * as React from 'react';
import { type SetState } from 'lib/types/hook-types';
import { type RelationshipButton } from 'lib/types/relationship-types';
import { type ThreadInfo } from 'lib/types/thread-types';
import type { UserInfo } from 'lib/types/user-types';
import ThreadSettingsRelationshipButton from './thread-settings-relationship-button.react';
import css from './thread-settings-relationship-tab.css';
type Props = {
+threadSettingsOperationInProgress: boolean,
+threadInfo: ThreadInfo,
tomekUnsubmitted
Not Done
Inline Actions

These props aren't used. Shouldn't this give a warning?

tomek: These props aren't used. Shouldn't this give a warning?
jacekAuthorUnsubmitted
Done
Inline Actions

Interesting. I thought flow should indicate warning for such variables.

jacek: Interesting. I thought flow should indicate warning for such variables.
ashoatUnsubmitted
Not Done
Inline Actions

There can be a good reason for having a prop that you never use: to force the component to rerender. For instance, we do something similar here

ashoat: There can be a good reason for having a prop that you never use: to force the component to…
+setErrorMessage: SetState<string>,
+relationshipButtons: $ReadOnlyArray<RelationshipButton>,
+otherUserInfo: UserInfo,
};
function ThreadSettingsRelationshipTab(props: Props): React.Node {
const { relationshipButtons, otherUserInfo, setErrorMessage } = props;
const buttons = React.useMemo(() => {
return relationshipButtons.map(action => {
return (
<ThreadSettingsRelationshipButton
key={action}
relationshipButton={action}
otherUserInfo={otherUserInfo}
setErrorMessage={setErrorMessage}
tomekUnsubmitted
Not Done
Inline Actions

In ThreadSettingsRelationshipButton setErrorMessage is string => void but here we use SetState<string>. Is flow ok with that? Is there a good reason for having different types?

tomek: In `ThreadSettingsRelationshipButton` `setErrorMessage` is `string => void` but here we use…
jacekAuthorUnsubmitted
Done
Inline Actions

Flow is ok here. I'll replace the type in ...Button with SetState<string>

jacek: Flow is ok here. I'll replace the type in `...Button` with `SetState<string>`
/>
);
});
}, [otherUserInfo, relationshipButtons, setErrorMessage]);
return <div className={css.container}>{buttons}</div>;
}
export default ThreadSettingsRelationshipTab;
View Options

keyserver/src/updaters/thread-permission-updaters.js

  • This file was added.
// @flow
import * as React from 'react';
import { type SetState } from 'lib/types/hook-types';
import { type RelationshipButton } from 'lib/types/relationship-types';
import { type ThreadInfo } from 'lib/types/thread-types';
import type { UserInfo } from 'lib/types/user-types';
import ThreadSettingsRelationshipButton from './thread-settings-relationship-button.react';
import css from './thread-settings-relationship-tab.css';
type Props = {
+threadSettingsOperationInProgress: boolean,
+threadInfo: ThreadInfo,
tomekUnsubmitted
Not Done
Inline Actions

These props aren't used. Shouldn't this give a warning?

tomek: These props aren't used. Shouldn't this give a warning?
jacekAuthorUnsubmitted
Done
Inline Actions

Interesting. I thought flow should indicate warning for such variables.

jacek: Interesting. I thought flow should indicate warning for such variables.
ashoatUnsubmitted
Not Done
Inline Actions

There can be a good reason for having a prop that you never use: to force the component to rerender. For instance, we do something similar here

ashoat: There can be a good reason for having a prop that you never use: to force the component to…
+setErrorMessage: SetState<string>,
+relationshipButtons: $ReadOnlyArray<RelationshipButton>,
+otherUserInfo: UserInfo,
};
function ThreadSettingsRelationshipTab(props: Props): React.Node {
const { relationshipButtons, otherUserInfo, setErrorMessage } = props;
const buttons = React.useMemo(() => {
return relationshipButtons.map(action => {
return (
<ThreadSettingsRelationshipButton
key={action}
relationshipButton={action}
otherUserInfo={otherUserInfo}
setErrorMessage={setErrorMessage}
tomekUnsubmitted
Not Done
Inline Actions

In ThreadSettingsRelationshipButton setErrorMessage is string => void but here we use SetState<string>. Is flow ok with that? Is there a good reason for having different types?

tomek: In `ThreadSettingsRelationshipButton` `setErrorMessage` is `string => void` but here we use…
jacekAuthorUnsubmitted
Done
Inline Actions

Flow is ok here. I'll replace the type in ...Button with SetState<string>

jacek: Flow is ok here. I'll replace the type in `...Button` with `SetState<string>`
/>
);
});
}, [otherUserInfo, relationshipButtons, setErrorMessage]);
return <div className={css.container}>{buttons}</div>;
}
export default ThreadSettingsRelationshipTab;