fix: mixing clientId & socketId in UserList (#7461)

excalidraw-app/collab/Collab.tsx

@@ -123,7 +123,7 @@ class Collab extends PureComponent<Props, CollabState> {
 
   private socketInitializationTimer?: number;
   private lastBroadcastedOrReceivedSceneVersion: number = -1;
-  private collaborators = new Map<string, Collaborator>();
+  private collaborators = new Map<SocketId, Collaborator>();
 
   constructor(props: Props) {
     super(props);
@@ -618,7 +618,7 @@ class Collab extends PureComponent<Props, CollabState> {
 
     this.portal.socket.on(
       WS_EVENTS.USER_FOLLOW_ROOM_CHANGE,
-      (followedBy: string[]) => {
+      (followedBy: SocketId[]) => {
         this.excalidrawAPI.updateScene({
           appState: { followedBy: new Set(followedBy) },
         });
@@ -795,7 +795,7 @@ class Collab extends PureComponent<Props, CollabState> {
     document.addEventListener(EVENT.VISIBILITY_CHANGE, this.onVisibilityChange);
   };
 
-  setCollaborators(sockets: string[]) {
+  setCollaborators(sockets: SocketId[]) {
     const collaborators: InstanceType<typeof Collab>["collaborators"] =
       new Map();
     for (const socketId of sockets) {

excalidraw-app/collab/Portal.tsx

@@ -10,6 +10,7 @@ import { ExcalidrawElement } from "../../packages/excalidraw/element/types";
 import { WS_EVENTS, FILE_UPLOAD_TIMEOUT, WS_SUBTYPES } from "../app_constants";
 import {
   OnUserFollowedPayload,
+  SocketId,
   UserIdleState,
 } from "../../packages/excalidraw/types";
 import { trackEvent } from "../../packages/excalidraw/analytics";
@@ -51,7 +52,7 @@ class Portal {
         /* syncAll */ true,
       );
     });
-    this.socket.on("room-user-change", (clients: string[]) => {
+    this.socket.on("room-user-change", (clients: SocketId[]) => {
       this.collab.setCollaborators(clients);
     });
 
@@ -186,7 +187,7 @@ class Portal {
       const data: SocketUpdateDataSource["IDLE_STATUS"] = {
         type: WS_SUBTYPES.IDLE_STATUS,
         payload: {
-          socketId: this.socket.id,
+          socketId: this.socket.id as SocketId,
           userState,
           username: this.collab.state.username,
         },
@@ -206,7 +207,7 @@ class Portal {
       const data: SocketUpdateDataSource["MOUSE_LOCATION"] = {
         type: WS_SUBTYPES.MOUSE_LOCATION,
         payload: {
-          socketId: this.socket.id,
+          socketId: this.socket.id as SocketId,
           pointer: payload.pointer,
           button: payload.button || "up",
           selectedElementIds:
@@ -232,7 +233,7 @@ class Portal {
       const data: SocketUpdateDataSource["USER_VISIBLE_SCENE_BOUNDS"] = {
         type: WS_SUBTYPES.USER_VISIBLE_SCENE_BOUNDS,
         payload: {
-          socketId: this.socket.id,
+          socketId: this.socket.id as SocketId,
           username: this.collab.state.username,
           sceneBounds: payload.sceneBounds,
         },

excalidraw-app/data/index.ts

@@ -22,6 +22,7 @@ import {
   AppState,
   BinaryFileData,
   BinaryFiles,
+  SocketId,
   UserIdleState,
 } from "../../packages/excalidraw/types";
 import { bytesToHexString } from "../../packages/excalidraw/utils";
@@ -117,7 +118,7 @@ export type SocketUpdateDataSource = {
   MOUSE_LOCATION: {
     type: WS_SUBTYPES.MOUSE_LOCATION;
     payload: {
-      socketId: string;
+      socketId: SocketId;
       pointer: { x: number; y: number; tool: "pointer" | "laser" };
       button: "down" | "up";
       selectedElementIds: AppState["selectedElementIds"];
@@ -127,7 +128,7 @@ export type SocketUpdateDataSource = {
   USER_VISIBLE_SCENE_BOUNDS: {
     type: WS_SUBTYPES.USER_VISIBLE_SCENE_BOUNDS;
     payload: {
-      socketId: string;
+      socketId: SocketId;
       username: string;
       sceneBounds: SceneBounds;
     };
@@ -135,7 +136,7 @@ export type SocketUpdateDataSource = {
   IDLE_STATUS: {
     type: WS_SUBTYPES.IDLE_STATUS;
     payload: {
-      socketId: string;
+      socketId: SocketId;
       userState: UserIdleState;
       username: string;
     };

packages/excalidraw/actions/actionNavigate.tsx

@@ -12,6 +12,7 @@ export const actionGoToCollaborator = register({
   trackEvent: { category: "collab" },
   perform: (_elements, appState, collaborator: Collaborator) => {
     if (
+      !collaborator.socketId ||
       appState.userToFollow?.socketId === collaborator.socketId ||
       collaborator.isCurrentUser
     ) {
@@ -28,7 +29,7 @@ export const actionGoToCollaborator = register({
       appState: {
         ...appState,
         userToFollow: {
-          socketId: collaborator.socketId!,
+          socketId: collaborator.socketId,
           username: collaborator.username || "",
         },
         // Close mobile menu

packages/excalidraw/components/App.tsx

@@ -3472,7 +3472,6 @@ class App extends React.Component<AppProps, AppState> {
   };
 
   private maybeUnfollowRemoteUser = () => {
-    console.warn("maybeUnfollowRemoteUser");
     if (this.state.userToFollow) {
       this.setState({ userToFollow: null });
     }

packages/excalidraw/components/LaserTool/LaserPathManager.ts

@@ -3,6 +3,7 @@ import { LaserPointer } from "@excalidraw/laser-pointer";
 import { sceneCoordsToViewportCoords } from "../../utils";
 import App from "../App";
 import { getClientColor } from "../../clients";
+import { SocketId } from "../../types";
 
 // decay time in milliseconds
 const DECAY_TIME = 1000;
@@ -88,7 +89,7 @@ type CollabolatorState = {
 
 export class LaserPathManager {
   private ownState: CollabolatorState;
-  private collaboratorsState: Map<string, CollabolatorState> = new Map();
+  private collaboratorsState: Map<SocketId, CollabolatorState> = new Map();
 
   private rafId: number | undefined;
   private isDrawing = false;

packages/excalidraw/components/UserList.tsx

@@ -14,51 +14,54 @@ import { t } from "../i18n";
 import { isShallowEqual } from "../utils";
 
 export type GoToCollaboratorComponentProps = [
-  SocketId,
+  ClientId,
   Collaborator,
   boolean,
   boolean,
 ];
 
+/** collaborator user id or socket id (fallback) */
+type ClientId = string & { _brand: "UserId" };
+
 const FIRST_N_AVATARS = 3;
 const SHOW_COLLABORATORS_FILTER_AT = 8;
 
 const ConditionalTooltipWrapper = ({
   shouldWrap,
   children,
-  socketId,
+  clientId,
   username,
 }: {
   shouldWrap: boolean;
   children: React.ReactNode;
   username?: string | null;
-  socketId: string;
+  clientId: ClientId;
 }) =>
   shouldWrap ? (
-    <Tooltip label={username || "Unknown user"} key={socketId}>
+    <Tooltip label={username || "Unknown user"} key={clientId}>
       {children}
     </Tooltip>
   ) : (
-    <React.Fragment key={socketId}>{children}</React.Fragment>
+    <React.Fragment key={clientId}>{children}</React.Fragment>
   );
 
 const renderCollaborator = ({
   actionManager,
   collaborator,
-  socketId,
+  clientId,
   withName = false,
   shouldWrapWithTooltip = false,
   isBeingFollowed,
 }: {
   actionManager: ActionManager;
   collaborator: Collaborator;
-  socketId: string;
+  clientId: ClientId;
   withName?: boolean;
   shouldWrapWithTooltip?: boolean;
   isBeingFollowed: boolean;
 }) => {
   const data: GoToCollaboratorComponentProps = [
-    socketId,
+    clientId,
     collaborator,
     withName,
     isBeingFollowed,
@@ -67,8 +70,8 @@ const renderCollaborator = ({
 
   return (
     <ConditionalTooltipWrapper
-      key={socketId}
+      key={clientId}
-      socketId={socketId}
+      clientId={clientId}
       username={collaborator.username}
       shouldWrap={shouldWrapWithTooltip}
     >
@@ -100,12 +103,13 @@ export const UserList = React.memo(
   ({ className, mobile, collaborators, userToFollow }: UserListProps) => {
     const actionManager = useExcalidrawActionManager();
 
-    const uniqueCollaboratorsMap = new Map<string, Collaborator>();
+    const uniqueCollaboratorsMap = new Map<ClientId, Collaborator>();
 
     collaborators.forEach((collaborator, socketId) => {
+      const userId = (collaborator.id || socketId) as ClientId;
       uniqueCollaboratorsMap.set(
         // filter on user id, else fall back on unique socketId
-        collaborator.id || socketId,
+        userId,
         { ...collaborator, socketId },
       );
     });
@@ -134,25 +138,25 @@ export const UserList = React.memo(
     );
 
     const firstNAvatarsJSX = firstNCollaborators.map(
-      ([socketId, collaborator]) =>
+      ([clientId, collaborator]) =>
         renderCollaborator({
           actionManager,
           collaborator,
-          socketId,
+          clientId,
           shouldWrapWithTooltip: true,
-          isBeingFollowed: socketId === userToFollow,
+          isBeingFollowed: collaborator.socketId === userToFollow,
         }),
     );
 
     return mobile ? (
       <div className={clsx("UserList UserList_mobile", className)}>
-        {uniqueCollaboratorsArray.map(([socketId, collaborator]) =>
+        {uniqueCollaboratorsArray.map(([clientId, collaborator]) =>
           renderCollaborator({
             actionManager,
             collaborator,
-            socketId,
+            clientId,
             shouldWrapWithTooltip: true,
-            isBeingFollowed: socketId === userToFollow,
+            isBeingFollowed: collaborator.socketId === userToFollow,
           }),
         )}
       </div>
@@ -205,13 +209,13 @@ export const UserList = React.memo(
                   <div className="UserList__hint">
                     {t("userList.hint.text")}
                   </div>
-                  {filteredCollaborators.map(([socketId, collaborator]) =>
+                  {filteredCollaborators.map(([clientId, collaborator]) =>
                     renderCollaborator({
                       actionManager,
                       collaborator,
-                      socketId,
+                      clientId,
                       withName: true,
-                      isBeingFollowed: socketId === userToFollow,
+                      isBeingFollowed: collaborator.socketId === userToFollow,
                     }),
                   )}
                 </div>

packages/excalidraw/types.ts

@@ -41,7 +41,7 @@ import { Merge, ValueOf } from "./utility-types";
 
 export type Point = Readonly<RoughPoint>;
 
-export type SocketId = string;
+export type SocketId = string & { _brand: "SocketId" };
 
 export type Collaborator = Readonly<{
   pointer?: CollaboratorPointer;
@@ -128,7 +128,7 @@ export type SidebarName = string;
 export type SidebarTabName = string;
 
 export type UserToFollow = {
-  socketId: string;
+  socketId: SocketId;
   username: string;
 };
 
@@ -296,7 +296,7 @@ export interface AppState {
   offsetLeft: number;
 
   fileHandle: FileSystemHandle | null;
-  collaborators: Map<string, Collaborator>;
+  collaborators: Map<SocketId, Collaborator>;
   showStats: boolean;
   currentChartType: ChartType;
   pasteDialog:
@@ -321,7 +321,7 @@ export interface AppState {
   /** the user's clientId & username who is being followed on the canvas */
   userToFollow: UserToFollow | null;
   /** the clientIds of the users following the current user */
-  followedBy: Set<string>;
+  followedBy: Set<SocketId>;
 }
 
 export type UIAppState = Omit<
@@ -474,7 +474,7 @@ export interface ExcalidrawProps {
 export type SceneData = {
   elements?: ImportedDataState["elements"];
   appState?: ImportedDataState["appState"];
-  collaborators?: Map<string, Collaborator>;
+  collaborators?: Map<SocketId, Collaborator>;
   commitToHistory?: boolean;
 };