Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove nin and ne operator usage in the SDK and the sample app #2672

Merged
merged 9 commits into from
Sep 18, 2024
25 changes: 12 additions & 13 deletions examples/SampleApp/src/hooks/usePaginatedUsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ export const usePaginatedUsers = (): PaginatedUsers => {
try {
queryInProgress.current = true;
const filter: UserFilters = {
id: {
$nin: [chatClient?.userID],
},
role: 'user',
};

Expand All @@ -143,7 +140,7 @@ export const usePaginatedUsers = (): PaginatedUsers => {
return;
}

const res = await chatClient?.queryUsers(
const { users } = await chatClient?.queryUsers(
filter,
{ name: 1 },
{
Expand All @@ -153,33 +150,35 @@ export const usePaginatedUsers = (): PaginatedUsers => {
},
);

if (!res?.users) {
queryInProgress.current = false;
return;
}
const usersWithoutClientUserId = users.filter((user) => user.id !== chatClient.userID);

// Dumb check to avoid duplicates
if (query === searchText && results.findIndex((r) => res?.users[0].id === r.id) > -1) {
if (
query === searchText &&
usersWithoutClientUserId.length > 0 &&
results.findIndex((r) => usersWithoutClientUserId[0].id === r.id) > -1
) {
queryInProgress.current = false;
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again unrelated but, this is quite confusing to me. What does this code do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove it imo. Seems useless now


setResults((r) => {
if (query !== searchText) {
return res?.users;
return usersWithoutClientUserId;
}
return r.concat(res?.users || []);
return r.concat(usersWithoutClientUserId);
});

if (res?.users.length < 10 && (offset.current === 0 || query === searchText)) {
if (usersWithoutClientUserId.length < 10 && (offset.current === 0 || query === searchText)) {
hasMoreResults.current = false;
}

if (!query && offset.current === 0) {
setInitialResults(res?.users || []);
setInitialResults(usersWithoutClientUserId);
}
} catch (e) {
// do nothing;
console.log('Error fetching users', e);
}
queryInProgress.current = false;
setLoading(false);
Expand Down
55 changes: 24 additions & 31 deletions package/src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import type React from 'react';

import dayjs from 'dayjs';
import EmojiRegex from 'emoji-regex';
import type { DebouncedFunc } from 'lodash';
import { DebouncedFunc } from 'lodash';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revert this one. I'm not sure if this change will lead to bundling the whole lodash library.

import debounce from 'lodash/debounce';
import type {
Channel,
ChannelMemberAPIResponse,
ChannelMemberResponse,
CommandResponse,
FormatMessageResponse,
MessageResponse,
Expand Down Expand Up @@ -152,14 +151,7 @@ const getMembers = <
channel: Channel<StreamChatGenerics>,
) => {
const members = channel.state.members;

return Object.values(members).length
? (
Object.values(members).filter((member) => member.user) as Array<
ChannelMemberResponse<StreamChatGenerics> & { user: UserResponse<StreamChatGenerics> }
>
).map((member) => member.user)
: [];
return members ? Object.values(members).map(({ user }) => user) : [];
};

const getWatchers = <
Expand All @@ -168,25 +160,28 @@ const getWatchers = <
channel: Channel<StreamChatGenerics>,
) => {
const watchers = channel.state.watchers;
return Object.values(watchers).length ? [...Object.values(watchers)] : [];
return watchers ? Object.values(watchers) : [];
};

const getMembersAndWatchers = <
StreamChatGenerics extends DefaultStreamChatGenerics = DefaultStreamChatGenerics,
>(
channel: Channel<StreamChatGenerics>,
) => {
const users = [...getMembers(channel), ...getWatchers(channel)];
const members = getMembers(channel);
const watchers = getWatchers(channel);
const users = [...members, ...watchers];

return Object.values(
users.reduce((acc, cur) => {
if (!acc[cur.id]) {
acc[cur.id] = cur;
}
// make sure we don't list users twice
const uniqueUsers = {} as Record<string, UserResponse<StreamChatGenerics>>;

users.forEach((user) => {
if (user && !uniqueUsers[user.id]) {
uniqueUsers[user.id] = user;
}
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but I think you can speed up this function and save one full object iteration (Object.values()) if you rewrite it like this:

const seenUsers = new Set<string>();
const uniqueUsers: User[] = [];
for (const user of users) {
  if (user && !seenUsers.has(user.id)) {
    uniqueUsers.push(user);
    seenUsers.add(user.id);
  }
}
return uniqueUsers;


return acc;
}, {} as { [key: string]: SuggestionUser<StreamChatGenerics> }),
);
return Object.values(uniqueUsers);
};

const queryMembers = async <
Expand Down Expand Up @@ -234,32 +229,30 @@ const queryUsers = async <
mentionAllAppUsersQuery?: MentionAllAppUsersQuery<StreamChatGenerics>;
} = {},
): Promise<void> => {
if (typeof query === 'string') {
if (!query) return;
try {
const {
limit = defaultAutoCompleteSuggestionsLimit,
mentionAllAppUsersQuery = defaultMentionAllAppUsersQuery,
} = options;

const filters = {
id: { $ne: client.userID },
$or: [{ id: { $autocomplete: query } }, { name: { $autocomplete: query } }],
...mentionAllAppUsersQuery?.filters,
};

if (query) {
// @ts-ignore
filters.$or = [{ id: { $autocomplete: query } }, { name: { $autocomplete: query } }];
}

const response = await client.queryUsers(
const { users } = await client.queryUsers(
// @ts-ignore
filters,
{ id: 1, ...mentionAllAppUsersQuery?.sort },
{ limit, ...mentionAllAppUsersQuery?.options },
);
const users: SuggestionUser<StreamChatGenerics>[] = [];
response.users.forEach((user) => isUserResponse(user) && users.push(user));
const usersWithoutClientUserId = users.filter((user) => user.id !== client.userID);
if (onReady && users) {
onReady(users);
onReady(usersWithoutClientUserId);
}
} catch (error) {
console.warn('Error querying users:', error);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we bubble this error up? How can one recover from an error that can happen here?

}
};

Expand Down
Loading