From 0170b989748bc7198ad83e90596f8bba63ab1c9a Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Fri, 23 May 2025 14:51:17 -0400 Subject: [PATCH] perf: Various improvements to collaborators facepile (#9281) --- app/components/Collaborators.tsx | 96 +++++++++++----- app/components/DocumentViews.tsx | 116 +++++++++++--------- app/scenes/Document/components/Insights.tsx | 2 +- 3 files changed, 129 insertions(+), 85 deletions(-) diff --git a/app/components/Collaborators.tsx b/app/components/Collaborators.tsx index 6e7444739d..c87d1f4fbe 100644 --- a/app/components/Collaborators.tsx +++ b/app/components/Collaborators.tsx @@ -34,55 +34,94 @@ function Collaborators(props: Props) { const [requestedUserIds, setRequestedUserIds] = useState([]); const { users, presence, ui } = useStores(); const { document } = props; + const { observingUserId } = ui; const documentPresence = presence.get(document.id); const documentPresenceArray = documentPresence ? Array.from(documentPresence.values()) : []; - const presentIds = documentPresenceArray.map((p) => p.userId); - const editingIds = documentPresenceArray - .filter((p) => p.isEditing) - .map((p) => p.userId); + // Use Set for O(1) lookups and stable references + const presentIds = useMemo( + () => new Set(documentPresenceArray.map((p) => p.userId)), + [documentPresenceArray] + ); + const editingIds = useMemo( + () => + new Set( + documentPresenceArray.filter((p) => p.isEditing).map((p) => p.userId) + ), + [documentPresenceArray] + ); // ensure currently present via websocket are always ordered first + // Memoize collaboratorIds as a Set for efficient lookup + const collaboratorIdsSet = useMemo( + () => new Set(document.collaboratorIds), + [document.collaboratorIds] + ); const collaborators = useMemo( () => orderBy( filter( users.all, (u) => - (presentIds.includes(u.id) || - document.collaboratorIds.includes(u.id)) && + (presentIds.has(u.id) || collaboratorIdsSet.has(u.id)) && !u.isSuspended ), - [(u) => presentIds.includes(u.id), "id"], + [(u) => presentIds.has(u.id), "id"], ["asc", "asc"] ), - [document.collaboratorIds, users.all, presentIds] + [collaboratorIdsSet, users.all, presentIds] ); // load any users we don't yet have in memory - useEffect(() => { - const ids = uniq([...document.collaboratorIds, ...presentIds]) - .filter((userId) => !users.get(userId)) - .sort(); + // Memoize ids to avoid unnecessary effect executions + const missingUserIds = useMemo( + () => + uniq([...document.collaboratorIds, ...Array.from(presentIds)]) + .filter((userId) => !users.get(userId)) + .sort(), + [document.collaboratorIds, presentIds, users] + ); - if (!isEqual(requestedUserIds, ids) && ids.length > 0) { - setRequestedUserIds(ids); - void users.fetchPage({ ids, limit: 100 }); + useEffect(() => { + if ( + !isEqual(requestedUserIds, missingUserIds) && + missingUserIds.length > 0 + ) { + setRequestedUserIds(missingUserIds); + void users.fetchPage({ ids: missingUserIds, limit: 100 }); } - }, [document, users, presentIds, document.collaboratorIds, requestedUserIds]); + }, [missingUserIds, requestedUserIds, users]); const popover = usePopoverState({ gutter: 0, placement: "bottom-end", }); + // Memoize onClick handler to avoid inline function creation + const handleAvatarClick = useCallback( + ( + collaboratorId: string, + isPresent: boolean, + isObserving: boolean, + isObservable: boolean + ) => + (ev: React.MouseEvent) => { + if (isObservable && isPresent) { + ev.preventDefault(); + ev.stopPropagation(); + ui.setObservingUser(isObserving ? undefined : collaboratorId); + } + }, + [ui] + ); + const renderAvatar = useCallback( ({ model: collaborator, ...rest }) => { - const isPresent = presentIds.includes(collaborator.id); - const isEditing = editingIds.includes(collaborator.id); - const isObserving = ui.observingUserId === collaborator.id; + const isPresent = presentIds.has(collaborator.id); + const isEditing = editingIds.has(collaborator.id); + const isObserving = observingUserId === collaborator.id; const isObservable = collaborator.id !== currentUserId; return ( @@ -96,21 +135,18 @@ function Collaborators(props: Props) { isCurrentUser={currentUserId === collaborator.id} onClick={ isObservable - ? (ev) => { - if (isPresent) { - ev.preventDefault(); - ev.stopPropagation(); - ui.setObservingUser( - isObserving ? undefined : collaborator.id - ); - } - } + ? handleAvatarClick( + collaborator.id, + isPresent, + isObserving, + isObservable + ) : undefined } /> ); }, - [presentIds, ui, currentUserId, editingIds] + [presentIds, editingIds, observingUserId, currentUserId, handleAvatarClick] ); return ( @@ -133,7 +169,7 @@ function Collaborators(props: Props) { )} - + {popover.visible && } ); diff --git a/app/components/DocumentViews.tsx b/app/components/DocumentViews.tsx index 528be2faaf..da077be23c 100644 --- a/app/components/DocumentViews.tsx +++ b/app/components/DocumentViews.tsx @@ -1,7 +1,7 @@ import compact from "lodash/compact"; import sortBy from "lodash/sortBy"; import { observer } from "mobx-react"; -import { useMemo } from "react"; +import { useMemo, useCallback } from "react"; import { useTranslation } from "react-i18next"; import { dateLocale, dateToRelative } from "@shared/utils/date"; import Document from "~/models/Document"; @@ -14,78 +14,86 @@ import useStores from "~/hooks/useStores"; type Props = { document: Document; - isOpen?: boolean; }; -function DocumentViews({ document, isOpen }: Props) { +function DocumentViews({ document }: Props) { const { t } = useTranslation(); const { views, presence } = useStores(); const user = useCurrentUser(); const locale = dateLocale(user.language); - const documentPresence = presence.get(document.id); const documentPresenceArray = documentPresence ? Array.from(documentPresence.values()) : []; - const presentIds = documentPresenceArray.map((p) => p.userId); - const editingIds = documentPresenceArray - .filter((p) => p.isEditing) - .map((p) => p.userId); + + // Use Set for O(1) lookups and stable references + const presentIds = useMemo( + () => new Set(documentPresenceArray.map((p) => p.userId)), + [documentPresenceArray] + ); + const editingIds = useMemo( + () => + new Set( + documentPresenceArray.filter((p) => p.isEditing).map((p) => p.userId) + ), + [documentPresenceArray] + ); // ensure currently present via websocket are always ordered first - const documentViews = views.inDocument(document.id); - const sortedViews = sortBy( - documentViews, - (view) => !presentIds.includes(view.userId) + const documentViews = useMemo( + () => views.inDocument(document.id), + [views, document.id] + ); + const sortedViews = useMemo( + () => sortBy(documentViews, (view) => !presentIds.has(view.userId)), + [documentViews, presentIds] ); const users = useMemo( () => compact(sortedViews.map((v) => v.user)), [sortedViews] ); - return ( - <> - {isOpen && ( - - aria-label={t("Viewers")} - items={users} - renderItem={(model) => { - const view = documentViews.find((v) => v.userId === model.id); - const isPresent = presentIds.includes(model.id); - const isEditing = editingIds.includes(model.id); - const subtitle = isPresent - ? isEditing - ? t("Currently editing") - : t("Currently viewing") - : t("Viewed {{ timeAgo }}", { - timeAgo: dateToRelative( - view ? Date.parse(view.lastViewedAt) : new Date(), - { - addSuffix: true, - locale, - } - ), - }); - return ( - - } - border={false} - small - /> - ); - }} + // Memoize renderItem for PaginatedList + const renderItem = useCallback( + (model: User) => { + const view = documentViews.find((v) => v.userId === model.id); + const isPresent = presentIds.has(model.id); + const isEditing = editingIds.has(model.id); + const subtitle = isPresent + ? isEditing + ? t("Currently editing") + : t("Currently viewing") + : t("Viewed {{ timeAgo }}", { + timeAgo: dateToRelative( + view ? Date.parse(view.lastViewedAt) : new Date(), + { + addSuffix: true, + locale, + } + ), + }); + return ( + + } + border={false} + small /> - )} - + ); + }, + [documentViews, presentIds, editingIds, t, locale] + ); + + return ( + + aria-label={t("Viewers")} + items={users} + renderItem={renderItem} + /> ); } diff --git a/app/scenes/Document/components/Insights.tsx b/app/scenes/Document/components/Insights.tsx index a3e6a250e2..f60a4a8fe1 100644 --- a/app/scenes/Document/components/Insights.tsx +++ b/app/scenes/Document/components/Insights.tsx @@ -194,7 +194,7 @@ function Insights() { {documentViews.length > 1 && ( - + )}