From 44b38347c4544462098cd53fe9c12293d79cbb9c Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 30 Jan 2026 10:20:43 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(frontend)=20fix=20broadcast=20stor?= =?UTF-8?q?e=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When going from one subdoc to another by example, the broadcast store could have difficulty to resync. This commit ensures that the broadcast store cleans up and resets its state when rerendering. It will stop as well triggering the action for the current user avoiding potential unecessary requests. --- CHANGELOG.md | 4 ++ .../__tests__/app-impress/doc-header.spec.ts | 60 ++++++++++++++++--- .../doc-management/hooks/useCollaboration.tsx | 5 +- .../impress/src/stores/useBroadcastStore.tsx | 55 ++++++++++++----- 4 files changed, 100 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81c1328f..282ce166 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to ## [Unreleased] +### Fixed + +🐛(frontend) fix broadcast store sync #1846 + ## [v4.5.0] - 2026-01-28 ### Added diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts index 34f0b4c4..4652f590 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts @@ -7,7 +7,12 @@ import { mockedDocument, verifyDocName, } from './utils-common'; -import { mockedAccesses, mockedInvitations } from './utils-share'; +import { + connectOtherUserToDoc, + mockedAccesses, + mockedInvitations, + updateShareLink, +} from './utils-share'; import { createRootSubPage, getTreeRow } from './utils-sub-pages'; test.beforeEach(async ({ page }) => { @@ -52,13 +57,54 @@ test.describe('Doc Header', () => { ).toBeVisible(); }); - test('it updates the title doc', async ({ page, browserName }) => { - await createDoc(page, 'doc-update', browserName, 1); - const docTitle = page.getByRole('textbox', { name: 'Document title' }); - await expect(docTitle).toBeVisible(); - await docTitle.fill('Hello World'); - await docTitle.blur(); + test('it updates the title doc and check the broadcast', async ({ + page, + browserName, + }) => { + const [docTitle] = await createDoc( + page, + 'doc-title-update', + browserName, + 1, + ); + await page.getByRole('button', { name: 'Share' }).click(); + await updateShareLink(page, 'Public', 'Editing'); + + const docUrl = page.url(); + + const { otherPage, cleanup } = await connectOtherUserToDoc({ + docUrl, + browserName, + withoutSignIn: true, + docTitle, + }); + + // Wait for other page to sync + await page.waitForTimeout(1000); + + await page.keyboard.press('Escape'); + const elTitle = page.getByRole('textbox', { name: 'Document title' }); + await expect(elTitle).toBeVisible(); + await elTitle.fill('Hello World'); + await elTitle.blur(); await verifyDocName(page, 'Hello World'); + + // Wait for other page to sync + await page.waitForTimeout(1000); + + // Check other user page + await verifyDocName(otherPage, 'Hello World'); + + const elTitleOther = otherPage.getByRole('textbox', { + name: 'Document title', + }); + await elTitleOther.fill('Hello Other World'); + await elTitleOther.blur(); + + // Check first user page + await verifyDocName(page, 'Hello Other World'); + + await cleanup(); }); test('it updates the title doc adding a leading emoji', async ({ diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useCollaboration.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useCollaboration.tsx index 14f3b715..553c0bbf 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useCollaboration.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useCollaboration.tsx @@ -8,7 +8,7 @@ import { Base64 } from '../types'; export const useCollaboration = (room?: string, initialContent?: Base64) => { const collaborationUrl = useCollaborationUrl(room); - const { setBroadcastProvider } = useBroadcastStore(); + const { setBroadcastProvider, cleanupBroadcast } = useBroadcastStore(); const { provider, createProvider, destroyProvider } = useProviderStore(); useEffect(() => { @@ -33,8 +33,9 @@ export const useCollaboration = (room?: string, initialContent?: Base64) => { useEffect(() => { return () => { if (room) { + cleanupBroadcast(); destroyProvider(); } }; - }, [destroyProvider, room]); + }, [destroyProvider, room, cleanupBroadcast]); }; diff --git a/src/frontend/apps/impress/src/stores/useBroadcastStore.tsx b/src/frontend/apps/impress/src/stores/useBroadcastStore.tsx index 2406d42d..03fb1985 100644 --- a/src/frontend/apps/impress/src/stores/useBroadcastStore.tsx +++ b/src/frontend/apps/impress/src/stores/useBroadcastStore.tsx @@ -5,7 +5,9 @@ import { create } from 'zustand'; interface BroadcastState { addTask: (taskLabel: string, action: () => void) => void; broadcast: (taskLabel: string) => void; + cleanupBroadcast: () => void; getBroadcastProvider: () => HocuspocusProvider | undefined; + handleProviderSync: () => void; provider?: HocuspocusProvider; setBroadcastProvider: (provider: HocuspocusProvider) => void; setTask: ( @@ -15,11 +17,12 @@ interface BroadcastState { ) => void; tasks: { [taskLabel: string]: { - task: Y.Array; + action: () => void; observer: ( event: Y.YArrayEvent, transaction: Y.Transaction, ) => void; + task: Y.Array; }; }; } @@ -27,7 +30,22 @@ interface BroadcastState { export const useBroadcastStore = create((set, get) => ({ provider: undefined, tasks: {}, - setBroadcastProvider: (provider) => set({ provider }), + setBroadcastProvider: (provider) => { + // Clean up old provider listeners + const oldProvider = get().provider; + if (oldProvider) { + oldProvider.off('synced', get().handleProviderSync); + } + + provider.on('synced', get().handleProviderSync); + set({ provider }); + }, + handleProviderSync: () => { + const tasks = get().tasks; + Object.entries(tasks).forEach(([taskLabel, { action }]) => { + get().addTask(taskLabel, action); + }); + }, getBroadcastProvider: () => { const provider = get().provider; if (!provider) { @@ -43,20 +61,16 @@ export const useBroadcastStore = create((set, get) => ({ return; } - const existingTask = get().tasks[taskLabel]; - if (existingTask) { - existingTask.task.unobserve(existingTask.observer); - get().setTask(taskLabel, existingTask.task, action); - return; - } - const task = provider.document.getArray(taskLabel); get().setTask(taskLabel, task, action); }, setTask: (taskLabel: string, task: Y.Array, action: () => void) => { let isInitializing = true; - const observer = () => { - if (!isInitializing) { + const observer = ( + _event: Y.YArrayEvent, + transaction: Y.Transaction, + ) => { + if (!isInitializing && !transaction.local) { action(); } }; @@ -73,16 +87,27 @@ export const useBroadcastStore = create((set, get) => ({ [taskLabel]: { task, observer, + action, }, }, })); }, broadcast: (taskLabel) => { + // Broadcast via Y.js provider (for users on the same document) const obTask = get().tasks?.[taskLabel]; - if (!obTask || !obTask.task) { - console.warn(`Task ${taskLabel} is not defined`); - return; + if (obTask?.task) { + obTask.task.push([`broadcast: ${taskLabel}`]); } - obTask.task.push([`broadcast: ${taskLabel}`]); + }, + cleanupBroadcast: () => { + const provider = get().provider; + if (provider) { + provider.off('synced', get().handleProviderSync); + } + + // Unobserve all document-specific tasks + Object.values(get().tasks).forEach(({ task, observer }) => { + task.unobserve(observer); + }); }, }));