Differential D7771 Diff 26731 lib/shared/search-utils.js

Changeset View

Standalone View

lib/shared/search-utils.js

// @flow		// @flow

		import * as React from 'react';

import SearchIndex from './search-index.js';		import SearchIndex from './search-index.js';
import {		import {
userIsMember,		userIsMember,
threadMemberHasPermission,		threadMemberHasPermission,
getContainingThreadID,		getContainingThreadID,
} from './thread-utils.js';		} from './thread-utils.js';
		import {
		searchMessages,
		searchMessagesActionTypes,
		} from '../actions/message-actions.js';
import genesis from '../facts/genesis.js';		import genesis from '../facts/genesis.js';
		import type { RawMessageInfo } from '../types/message-types.js';
import { userRelationshipStatus } from '../types/relationship-types.js';		import { userRelationshipStatus } from '../types/relationship-types.js';
import { threadPermissions } from '../types/thread-permission-types.js';		import { threadPermissions } from '../types/thread-permission-types.js';
import { type ThreadType, threadTypes } from '../types/thread-types-enum.js';		import { type ThreadType, threadTypes } from '../types/thread-types-enum.js';
import { type ThreadInfo } from '../types/thread-types.js';		import { type ThreadInfo } from '../types/thread-types.js';
import type { AccountUserInfo, UserListItem } from '../types/user-types.js';		import type { AccountUserInfo, UserListItem } from '../types/user-types.js';
		import {
		useServerCall,
		useDispatchActionPromise,
		} from '../utils/action-utils.js';

const notFriendNotice = 'not friend';		const notFriendNotice = 'not friend';

function getPotentialMemberItems(		function getPotentialMemberItems(
text: string,		text: string,
userInfos: { +[id: string]: AccountUserInfo },		userInfos: { +[id: string]: AccountUserInfo },
searchIndex: SearchIndex,		searchIndex: SearchIndex,
excludeUserIDs: $ReadOnlyArray<string>,		excludeUserIDs: $ReadOnlyArray<string>,
▲ Show 20 Lines • Show All 141 Lines • ▼ Show 20 Lines	({
if (alertTitle) {		if (alertTitle) {
result = { ...result, alertTitle, alertText };		result = { ...result, alertTitle, alertText };
}		}
return result;		return result;
},		},
);		);
}		}

export { getPotentialMemberItems, notFriendNotice };		function useSearchMessages(
		query: string,
		threadID: string,
		cursor?: string,
		ashoatUnsubmitted Not Done Inline Actions It never makes sense to have an optional parameter before a required one. Please avoid this going forward! ashoat: It never makes sense to have an optional parameter before a required one. Please avoid this…
		callback: (messages: $ReadOnlyArray<RawMessageInfo>) => void,
		ashoatUnsubmitted Not Done Inline Actions Can you give this a better name? Maybe `onResultsReceived`? ashoat: Can you give this a better name? Maybe `onResultsReceived`?
		): boolean {
		const [endReached, setEndReached] = React.useState(false);

		const callSearchMessages = useServerCall(searchMessages);
		const dispatchActionPromise = useDispatchActionPromise();

		React.useEffect(() => {
		const searchMessagesPromise = (async () => {
		if (query === '') {
		setEndReached(true);
		return;
		}
		const { messages, endReached: end } = await callSearchMessages({
		query,
		threadID,
		cursor,
		});
		setEndReached(end);
		callback(messages);
		})();

		dispatchActionPromise(searchMessagesActionTypes, searchMessagesPromise);
		}, [
		callSearchMessages,
		query,
		threadID,
		cursor,
		dispatchActionPromise,
		callback,
		ashoatUnsubmitted Not Done Inline Actions This is significantly improved! I'm still a little concerned about the separation of logic between D7771 / D7774 / D7775. We have three files that are managing the state here, which is confusing and hard to follow. As an example, resetting the search results to empty when the query changes is handled in `MessageSearch` in D7774. But since your code here will re-query for results if eg. `callSearchMessages` or `dispatchActionPromise` changes, we should arguably be resetting the search results to empty in those cases too. Luckily those don't change often (or at all?), but it feels like the effect here is tightly coupled with the effect in `MessageSearch`, and if we added a new parameter to the dep list here, we might need to update the effect in `MessageSearch`. For that reason, I wonder if it would make sense to have these effects in the same file, right next to each other. Perhaps when you said "This diff will need to be dropped if those changes are accepted." in D7775, you were saying your plan is to unify some of this logic? ashoat: This is significantly improved! I'm still a little concerned about the separation of logic…
		inkaAuthorUnsubmitted Done Inline Actions We have three files that are managing the state here, which is confusing and hard to follow.v Three files? After these changes there are only two - search-utils and message-search. Do you mean the context? For that reason, I wonder if it would make sense to have these effects in the same file, right next to each other. I will return a callback from here and move the effect to `MessageSearch`. This should fix the issue. The callback will depend on `callSearchMessages` and `dispatchActionPromise`, and I will have the effect that clears the results depend on this callback. inka: > We have three files that are managing the state here, which is confusing and hard to follow.v…
		]);

		return endReached;
		}

		export { getPotentialMemberItems, notFriendNotice, useSearchMessages };