Просмотр исходного кода

fix(editor): filter undefined awareness entries and remove getStates mutation

The emitEditorList function produced undefined values for clients whose
awareness state had not yet included an editors field, causing
EditingUserList to disappear. The updateAwarenessHandler also incorrectly
mutated the Yjs-managed awareness map via getStates().delete(), which
Yjs already handles before emitting the update event.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Yuki Takei 1 неделя назад
Родитель
Сommit
3980691109

+ 142 - 0
packages/editor/src/client/stores/use-collaborative-editor-mode.spec.ts

@@ -0,0 +1,142 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import type { EditingClient } from '../../interfaces';
+
+/**
+ * Unit tests for awareness state handling logic extracted from
+ * use-collaborative-editor-mode.ts.
+ *
+ * These tests cover:
+ * - Task 1.1: undefined awareness entries are filtered before onEditorsUpdated is called
+ * - Task 1.2: awareness.getStates().delete() is NOT called for removed clients
+ * - Task 4.1: Requirements 1.1, 1.2, 1.4
+ */
+
+// ---------------------------------------------------------------------------
+// Helpers — minimal stubs that replicate the logic under test
+// ---------------------------------------------------------------------------
+
+type AwarenessState = { editors?: EditingClient };
+
+/** Replicates the FIXED emitEditorList logic */
+function emitEditorList(
+  states: Map<number, AwarenessState>,
+  onEditorsUpdated: (list: EditingClient[]) => void,
+): void {
+  const clientList = Array.from(states.values())
+    .map((v) => v.editors)
+    .filter((v): v is EditingClient => v != null);
+  onEditorsUpdated(clientList);
+}
+
+/** Replicates the FIXED updateAwarenessHandler logic */
+function updateAwarenessHandler(
+  update: { added: number[]; updated: number[]; removed: number[] },
+  awareness: { getStates: () => Map<number, AwarenessState> },
+  onEditorsUpdated: (list: EditingClient[]) => void,
+): void {
+  // Task 1.2: No .delete() call for removed client IDs
+  emitEditorList(awareness.getStates(), onEditorsUpdated);
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+const validClient: EditingClient = {
+  clientId: 1,
+  name: 'Alice',
+  color: '#ff0000',
+  colorLight: '#ff000033',
+};
+
+describe('emitEditorList', () => {
+  describe('Task 1.1 / 1.4 — filter undefined awareness entries', () => {
+    it('passes only valid EditingClient entries to onEditorsUpdated', () => {
+      const states = new Map<number, AwarenessState>([
+        [1, { editors: validClient }],
+        [2, {}], // no editors field
+        [3, { editors: undefined }], // editors explicitly undefined
+      ]);
+      const onEditorsUpdated = vi.fn();
+
+      emitEditorList(states, onEditorsUpdated);
+
+      expect(onEditorsUpdated).toHaveBeenCalledOnce();
+      expect(onEditorsUpdated).toHaveBeenCalledWith([validClient]);
+    });
+
+    it('calls onEditorsUpdated with an empty array when no state has editors', () => {
+      const states = new Map<number, AwarenessState>([
+        [1, {}],
+        [2, { editors: undefined }],
+      ]);
+      const onEditorsUpdated = vi.fn();
+
+      emitEditorList(states, onEditorsUpdated);
+
+      expect(onEditorsUpdated).toHaveBeenCalledWith([]);
+    });
+
+    it('passes all valid entries when every state has editors', () => {
+      const anotherClient: EditingClient = {
+        clientId: 2,
+        name: 'Bob',
+        color: '#0000ff',
+        colorLight: '#0000ff33',
+      };
+      const states = new Map<number, AwarenessState>([
+        [1, { editors: validClient }],
+        [2, { editors: anotherClient }],
+      ]);
+      const onEditorsUpdated = vi.fn();
+
+      emitEditorList(states, onEditorsUpdated);
+
+      expect(onEditorsUpdated).toHaveBeenCalledWith([
+        validClient,
+        anotherClient,
+      ]);
+    });
+  });
+});
+
+describe('updateAwarenessHandler', () => {
+  describe('Task 1.2 — no direct mutation of awareness.getStates()', () => {
+    it('does NOT call .delete() on the awareness states map for removed clients', () => {
+      const deleteSpy = vi.fn();
+      const states = new Map<number, AwarenessState>([
+        [1, { editors: validClient }],
+      ]);
+      states.delete = deleteSpy;
+
+      const awareness = { getStates: () => states };
+      const onEditorsUpdated = vi.fn();
+
+      updateAwarenessHandler(
+        { added: [], updated: [], removed: [99] },
+        awareness,
+        onEditorsUpdated,
+      );
+
+      expect(deleteSpy).not.toHaveBeenCalled();
+    });
+
+    it('still calls onEditorsUpdated after a removed event', () => {
+      const states = new Map<number, AwarenessState>([
+        [1, { editors: validClient }],
+      ]);
+      const awareness = { getStates: () => states };
+      const onEditorsUpdated = vi.fn();
+
+      updateAwarenessHandler(
+        { added: [], updated: [], removed: [99] },
+        awareness,
+        onEditorsUpdated,
+      );
+
+      expect(onEditorsUpdated).toHaveBeenCalledOnce();
+      expect(onEditorsUpdated).toHaveBeenCalledWith([validClient]);
+    });
+  });
+});

+ 5 - 11
packages/editor/src/client/stores/use-collaborative-editor-mode.ts

@@ -69,13 +69,10 @@ export const useCollaborativeEditorMode = (
 
     const emitEditorList = () => {
       if (onEditorsUpdated == null) return;
-      const clientList: EditingClient[] = Array.from(
-        awareness.getStates().values(),
-        (value) => value.editors,
-      );
-      if (Array.isArray(clientList)) {
-        onEditorsUpdated(clientList);
-      }
+      const clientList = Array.from(awareness.getStates().values())
+        .map((value) => value.editors)
+        .filter((v): v is EditingClient => v != null);
+      onEditorsUpdated(clientList);
     };
 
     const providerSyncHandler = (isSync: boolean) => {
@@ -84,14 +81,11 @@ export const useCollaborativeEditorMode = (
 
     _provider.on('sync', providerSyncHandler);
 
-    const updateAwarenessHandler = (update: {
+    const updateAwarenessHandler = (_update: {
       added: number[];
       updated: number[];
       removed: number[];
     }) => {
-      for (const clientId of update.removed) {
-        awareness.getStates().delete(clientId);
-      }
       emitEditorList();
     };
 

+ 12 - 0
packages/editor/vitest.config.ts

@@ -0,0 +1,12 @@
+import tsconfigPaths from 'vite-tsconfig-paths';
+import { defineConfig } from 'vitest/config';
+
+export default defineConfig({
+  plugins: [tsconfigPaths()],
+  test: {
+    clearMocks: true,
+    globals: true,
+    environment: 'happy-dom',
+    include: ['**/*.{spec,integ}.{ts,tsx}'],
+  },
+});